[libvirt] [PATCH] Add PCI sysfs reset access

I'm proposing we make use of $PCIDIR/reset in qemu-kvm to reset devices on VM reset. We need to add it to libvirt's list of files that get ownership for device assignment. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- src/util/pci.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 095ad3f..8d2dbb0 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1349,11 +1349,13 @@ int pciDeviceFileIterate(pciDevice *dev, while ((ent = readdir(dir)) != NULL) { /* Device assignment requires: - * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, $PCIDIR/rom + * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, + * $PCIDIR/rom, $PCIDIR/reset */ if (STREQ(ent->d_name, "config") || STRPREFIX(ent->d_name, "resource") || - STREQ(ent->d_name, "rom")) { + STREQ(ent->d_name, "rom") || + STREQ(ent->d_name, "reset")) { if (virAsprintf(&file, "%s/%s", pcidir, ent->d_name) < 0) { virReportOOMError(); goto cleanup;

On 03/17/2011 02:26 PM, Alex Williamson wrote:
I'm proposing we make use of $PCIDIR/reset in qemu-kvm to reset devices on VM reset. We need to add it to libvirt's list of files that get ownership for device assignment.
Signed-off-by: Alex Williamson <alex.williamson@redhat.com> ---
src/util/pci.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c index 095ad3f..8d2dbb0 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1349,11 +1349,13 @@ int pciDeviceFileIterate(pciDevice *dev,
while ((ent = readdir(dir)) != NULL) { /* Device assignment requires: - * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, $PCIDIR/rom + * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, + * $PCIDIR/rom, $PCIDIR/reset
Hmm, in the case of hot-plug, we are already passing the config fd via SCM_RIGHTS rather than having qemu directly open it. Should we be passing all of these files via SCM_RIGHTS?
*/ if (STREQ(ent->d_name, "config") || STRPREFIX(ent->d_name, "resource") || - STREQ(ent->d_name, "rom")) { + STREQ(ent->d_name, "rom") || + STREQ(ent->d_name, "reset")) {
At any rate, this looks reasonable to me, so ACK, and I'll push it shortly. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Thu, 2011-03-17 at 14:34 -0600, Eric Blake wrote:
On 03/17/2011 02:26 PM, Alex Williamson wrote:
I'm proposing we make use of $PCIDIR/reset in qemu-kvm to reset devices on VM reset. We need to add it to libvirt's list of files that get ownership for device assignment.
Signed-off-by: Alex Williamson <alex.williamson@redhat.com> ---
src/util/pci.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c index 095ad3f..8d2dbb0 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1349,11 +1349,13 @@ int pciDeviceFileIterate(pciDevice *dev,
while ((ent = readdir(dir)) != NULL) { /* Device assignment requires: - * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, $PCIDIR/rom + * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, + * $PCIDIR/rom, $PCIDIR/reset
Hmm, in the case of hot-plug, we are already passing the config fd via SCM_RIGHTS rather than having qemu directly open it. Should we be passing all of these files via SCM_RIGHTS?
I believe config is the only file that does some permission checks on open that requires those rights. The rest seem to be strictly file permission access.
*/ if (STREQ(ent->d_name, "config") || STRPREFIX(ent->d_name, "resource") || - STREQ(ent->d_name, "rom")) { + STREQ(ent->d_name, "rom") || + STREQ(ent->d_name, "reset")) {
At any rate, this looks reasonable to me, so ACK, and I'll push it shortly.
Thanks! Alex

On Thu, Mar 17, 2011 at 02:26:36PM -0600, Alex Williamson wrote:
I'm proposing we make use of $PCIDIR/reset in qemu-kvm to reset devices on VM reset. We need to add it to libvirt's list of files that get ownership for device assignment.
What does access to the 'reset' file allow you todo ? If we're to allow access, it must only allow for Function Level Resets. We don't want QEMU doing bus level resets, for example, since that could impact other VMs. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Fri, 2011-03-18 at 10:18 +0000, Daniel P. Berrange wrote:
On Thu, Mar 17, 2011 at 02:26:36PM -0600, Alex Williamson wrote:
I'm proposing we make use of $PCIDIR/reset in qemu-kvm to reset devices on VM reset. We need to add it to libvirt's list of files that get ownership for device assignment.
What does access to the 'reset' file allow you todo ? If we're to allow access, it must only allow for Function Level Resets. We don't want QEMU doing bus level resets, for example, since that could impact other VMs.
Writing 1 to the reset file calls pci_reset_function(), which is the same function kvm calls to clear the state of the device when it's first assigned. This will try device specific resets, flr reset, pm reset, and finally bus reset, but only if this device is the only device on the bus. Thanks, Alex
participants (3)
-
Alex Williamson
-
Daniel P. Berrange
-
Eric Blake