[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 09/13] libxl: convert libxl_device_nic_add to an async operation

Ian Jackson wrote:
Roger Pau Monne writes ("[PATCH 09/13] libxl: convert libxl_device_nic_add to an 
async operation"):
This patch converts libxl_device_nic_add to an ao operation that
waits for device backend to reach state XenbusStateInitWait and then
marks the operation as completed. This is not really useful now, but
will be used by latter patches that will launch hotplug scripts after
we reached the desired xenbus state.

Why do you serialise vif and vbd initialisation ?  Would anything go
wrong if you did them both at once ?

disk needs to be added before Qemu is launched, on the other hand nics need to be added after qemu is launched, so we need to add them at different moments during the domain creation, it's a PITA.

+    /* Plug nic interfaces */
+    if (!ret&&  d_config->num_vifs>  0) {
+        /* Attach nics */
+        GCNEW_ARRAY(dcs->devices, d_config->num_vifs);
+        dcs->num_devices = d_config->num_vifs;
+        for (i = 0; i<  d_config->num_vifs; i++) {
+            libxl__init_ao_device(&dcs->devices[i], ao,&dcs->devices);
+            dcs->devices[i].callback = domcreate_nics_connected;
+            libxl__device_nic_add(egc, domid,&d_config->vifs[i],

Ah this pattern again....

-int libxl__device_nic_add(libxl__gc *gc, uint32_t domid, libxl_device_nic *nic)
+void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
+                           libxl_device_nic *nic, libxl__ao_device *aorm)
+    STATE_AO_GC(aorm->ao);
      flexarray_t *front;
      flexarray_t *back;
-    libxl__device device;
+    libxl__device *device;

You might want to consider a pre-patch which changes this, and
libxl__device_disk_add's, as follows:
   -    libxl__device device;
   +    libxl__device device[1];

That would get rid of the noise from this patch.  Up to you, though;
because it's separated out textually doesn't make the diff unreadable
like it is.

@@ -845,9 +851,11 @@ static void spawn_stub_disk_connected(libxl__egc *egc, 
libxl__ao_device *aorm)

      for (i = 0; i<  dm_config->num_vifs; i++) {
-        ret = libxl_device_nic_add(ctx, dm_domid,&dm_config->vifs[i]);
-        if (ret)
-            goto out;
+        /* We have to init the nic here, because we still haven't
+         * called libxl_device_nic_add at this point, but qemu needs
+         * the nic information to be complete.
+         */
+        libxl__device_nic_setdefault(gc,&dm_config->vifs[i]);

This is really too much repetition now.  Can we not have a common "add
devices" function which is called once for the main domain and once
for the stubdom ?

I've added a functions that does the libxl_device_disk_add loop, and another one for the nics. Although this line you are quoting is not adding a nic, it is just filling the needed values so we can launch Qemu correctly.

In the future we'll need that if we have more service domains, too.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.