Re: [PATCH for-4.5] libxl: remove existence check for PCI device hotplug

[Ian Campbell <Ian.Campbell@citrix.com>]

On Mon, 2014-11-17 at 12:10 +0000, Wei Liu wrote:
> The existence check is to make sure a device is not added to a guest
> multiple times.
> 
> PCI device backend path has different rules from vif, disk etc. For
> example:
> /local/domain/0/backend/pci/9/0/dev-1/0000:03:10.1
> /local/domain/0/backend/pci/9/0/key-1/0000:03:10.1
> /local/domain/0/backend/pci/9/0/dev-2/0000:03:10.2
> /local/domain/0/backend/pci/9/0/key-2/0000:03:10.2
> 
> The devid for PCI devices is hardcoded 0.

FWIW I think "devid" here is effectively the PCI bus ID, and no
toolstack I know of has ever supported multiple PCI buses. In theory it
would be possible though. This means that the 0 corresponds to the
"0000:" too, I think.

This doesn't invalidate your reasoning, just FYI.

>  libxl__device_exists only
> checks up to /local/.../9/0 so it always returns true even the device is
> assignable.
> 
> Remove invocation of libxl__device_exists. We're sure at this point that
> the PCI device is assignable (hence no xenstore entry or JSON entry).
> The check is done before hand. For HVM guest it's done by calling
> xc_test_assign_device and for PV guest it's done by calling
> pciback_dev_is_assigned.
> 
> Reported-by: Li, Liang Z <liang.z.li@intel.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Konrad Wilk <konrad.wilk@oracle.com>
> ---
> This patch fixes a regression in 4.5.
> 
> The risk is that I misunderstood semantics of xc_test_assign_device and
> pciback_dev_is_assigned and end up adding several entries to JSON config
> template. But if the assignable tests are incorrect I think we have a
> bigger problem to worry about than duplicated entries in JSON template.
> 
> It would be good for someone to have PCI hotplug setup to run a quick test.  I
> think Liang confirmed (indrectly) that xc_test_assign_device worked well for
> him so I think there's won't be multiple JSON template entries for HVM guests.
> However PV side still remains to be tested.

I don't think you need any kind of special h/w support to do PV pci
hotplug to guests, do you?

Ian.