Re: [PATCH v10 16/19] libxl: call hotplug scripts for nic devices from libxl

[Roger Pau Monne <roger.pau@citrix.com>]

Ian Campbell wrote:
>> +        switch (nictype) {
>> +        case LIBXL_NIC_TYPE_VIF_IOEMU:
>> +            env[nr++] = "INTERFACE";
>> +            env[nr++] = libxl__strdup(gc, libxl__device_nic_devname(gc,
>> +                                                      dev->domid, dev->devid,
>> +                                                      LIBXL_NIC_TYPE_VIF_IOEMU));
> 
> Is this fall-through of the case deliberate? If so then a comment would
> be good.
> 
> However I'm not sure about this. It seems like the hotplug scripts are
> only called once in the VIF_IOEMU case? I would expect one call for the
> PV VIF device and one for the TAP device? Perhaps I'm just missing the
> location of the other one? Is this "num_exec" variable involved in some
> way? But you don't propagate that to get_hotplug_env() and therefore
> include *both* sets of env vars?

Yes, we perform two calls, one for the vif device and one for the tap
device. I agree that this approach is not really clear, what might be
more suitable is to return arrays of hotplug execution items from
get_hotplug_script_info:

args =
	{
		{"/etc/xen/script/foo", "bar"},
		{"/etc/xen/script/foo2", "bar2"},
		NULL
	};

And the same with env, so we keep executing items until we reach args[i]
== NULL.

> In any case something related seems to be broken for guests which use a
> stub domain and use vifname=foo. What I see in that case is the script
> called three times...
>      1. For the PV interface of the stub domain (LIBXL_NIC_TYPE_VIF),
>         renames vifX+1.0 -> foo-emu (*)
>      2. For the PV interface of the real domain
>         (LIBXL_NIC_TYPE_VIF_IOEMU part 1), renames vifX.0 -> foo
>      3. For the emu interface of the real domain
>         (LIBXL_NIC_TYPE_VIF_IOEMU part 2) renames vifX.0 -> foo-emu
> 
> The rename in #3 fails, because foo-emu already exists. But in fact this
> call to the script should never happen for a domain with a stub domain
> since the emulated device in this case is within the stub domain, not
> the driver domain.

As talked, the correct fix for this seems to detect if the guest has a
stubdom and return 0 on the second call to get_hotplug_script_info, to
prevent the execution of #3.

> (*) This is with the patch I posted earlier to rename the stub domain's
> vif with a -emu suffix. Without that then the clash is at #2 instead.