Re: [libvirt] [PATCH] util: disallow resetting a pci-bridge device

On 01/17/2017 09:51 AM, Shivaprasad G Bhat wrote: > It is distructive to attempt a reset on pci-bridge, the host can crash. > The bridges won't contain any guest data and neither they can be > passed through using vfio/stub. So, no point in allowing a reset for them. > > Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; > --- > src/util/virpci.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/src/util/virpci.c b/src/util/virpci.c > index 0601f49..860f7aa 100644 > --- a/src/util/virpci.c > +++ b/src/util/virpci.c > @@ -933,6 +933,16 @@ virPCIDeviceReset(virPCIDevicePtr dev, > This function is a nice centralized place to perform this check, but it could lead to an odd error message if someone tried to assign a PCI bridge device to a guest - instead of logging "PCI bridge devices can't be assigned to guests" (or something like that) it would tell us that we can't *reset* the device. Maybe that can be remedied by doing an additional check at an earlier stage during virHostdevPreparePCIDevices(). char *drvName = NULL; > int ret = -1; > int fd = -1; > + int hdrType = -1; > + > + if (virPCIGetHeaderType(dev, &hdrType) < 0) > + return -1; > + > + if (hdrType == VIR_PCI_HEADER_PCI_BRIDGE) { > I think we can only reset (or assign) endpoint devices, so how about using "hdrType != VIR_PCI_HEADER_ENDPOINT", and then having the error message say "Invalid attempt to reset non-endpoint PCI device at xxxx:xx:xx.x. Only PCI endpoint devices can be reset" + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Resetting a pci-bridge device is not > allowed")); > Using "pci-bridge" could be confusing, since that's the exact name of a particular emulated device in libvirt. The error message should include the PCI address of the device, and (as suggested above) just say that it isn't an endpoint device, so we can't reset it. + return -1; > + } > if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) { > virReportError(VIR_ERR_INTERNAL_ERROR, > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list