- Devel - libvirt List Archives

[libvirt] [PATCH] log: fix false oom error message
by Nikolay Shirokovskiy 12 Sep '16

12 Sep '16

In case filter is new one an oom error message is reported. This bug is introduced in 51b2606f. However this error does not get into the log in the default configuration. To trigger it one need to specify log outputs in LIBVIRT_LOG_OUTPUTS variable. --- src/util/virlog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 06f9a60..f5b88b6 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -335,11 +335,12 @@ virLogDefineFilter(const char *match, goto cleanup; virLogFiltersSerial++; + ret = virLogNbFilters - 1; cleanup: virLogUnlock(); if (ret < 0) virReportOOMError(); - return virLogNbFilters; + return ret; } /** -- 1.8.3.1

2 1

[libvirt] [PATCH] qemu: Don't use query-migrate on destination
by Jiri Denemark 12 Sep '16

12 Sep '16

When migration fails, we need to poke QEMU monitor to check for a reason of the failure. We did this using query-migrate QMP command, which is not supposed to return any meaningful result on the destination side. Thus if the monitor was still functional when we detected the migration failure, parsing the answer from query-migrate always failed with the following error message: "info migration reply was missing return status" This irrelevant message was then used as the reason for the migration failure replacing any message we might have had. Let's use harmless query-status for poking the monitor to make sure we only get an error if the monitor connection is broken. https://bugzilla.redhat.com/show_bug.cgi?id=1374613 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_domain.c | 20 ++++++++++++++++++++ src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_migration.c | 6 +----- src/qemu/qemu_monitor.c | 8 ++++++++ src/qemu/qemu_monitor.h | 1 + 5 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fd79390..fb766d0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6127,3 +6127,23 @@ qemuDomainVcpuPersistOrder(virDomainDefPtr def) } } } + + +int +qemuDomainCheckMonitor(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + ret = qemuMonitorCheck(priv->mon); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + return ret; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 13c0372..a1404d0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -728,4 +728,8 @@ bool qemuDomainVcpuHotplugIsInOrder(virDomainDefPtr def) void qemuDomainVcpuPersistOrder(virDomainDefPtr def) ATTRIBUTE_NONNULL(1); +int qemuDomainCheckMonitor(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 07f18db..e734816 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -6208,14 +6208,10 @@ qemuMigrationFinish(virQEMUDriverPtr driver, } if (retcode != 0) { - qemuDomainJobInfo info; - /* Check for a possible error on the monitor in case Finish was called * earlier than monitor EOF handler got a chance to process the error */ - qemuMigrationFetchJobStatus(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_IN, - &info); + qemuDomainCheckMonitor(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN); goto endjob; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4171914..1fdee3a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1617,6 +1617,14 @@ qemuMonitorStopCPUs(qemuMonitorPtr mon) int +qemuMonitorCheck(qemuMonitorPtr mon) +{ + bool running; + return qemuMonitorGetStatus(mon, &running, NULL); +} + + +int qemuMonitorGetStatus(qemuMonitorPtr mon, bool *running, virDomainPausedReason *reason) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b838725..255fff2 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -382,6 +382,7 @@ typedef enum { VIR_ENUM_DECL(qemuMonitorVMStatus) int qemuMonitorVMStatusToPausedReason(const char *status); +int qemuMonitorCheck(qemuMonitorPtr mon); int qemuMonitorGetStatus(qemuMonitorPtr mon, bool *running, virDomainPausedReason *reason) -- 2.10.0

2 1

[libvirt] [PATCH 0/2] A couple of volume encryption patches
by John Ferlan 12 Sep '16

12 Sep '16

I'm assume there will be a couple of bz's on these... Patch 1 fixes a problem where the vol-dumpxml would not list the <secret> element from a volume in a pool after the initial creation and a pool refresh (or libvirtd restart). This issue was missed due to rewriting the LUKS support to not use it's own "new" secret type (key), but rather use the existing "volume" secret type. Patch 2 fixes a problem where QCOW2 (or QCOW1) encrypted volume would lose the <encryption> and <secret> after a volume refresh. Details are in the patch. The issue is rooted in proper detection of the volume type during the virStorageFileGetMetadataFromBuf call. John Ferlan (2): storage: Need to refresh secret for luks volume after volume refresh storage: Need to properly read the crypt offset value src/storage/storage_backend_fs.c | 16 ++++++++++------ src/util/virstoragefile.c | 7 +++++-- 2 files changed, 15 insertions(+), 8 deletions(-) -- 2.7.4

2 3

[libvirt] [PATCH 0/2] try harder to get dest qemu errors on migation
by Nikolay Shirokovskiy 12 Sep '16

12 Sep '16

Hi, all. In case migration fails due to destination qemu exits unexpectedly user recevies the qemu log in the error message. Unfortunately log is truncated and the most interesting part is missed (below is the example of such a log [1]). Actually for the most cases the first patch will be enough to fix the issue. Originally I thought the problem is qemu logging and reading the log are not in sync (which is true) so I tried to fix it as well in the second patch. [1] log example: CPU Reset (CPU 0) EAX=00000000 EBX=00000000 ECX=00000000 EDX=00000000 ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000 EIP=00000000 EFL=00000000 [-------] CPL=0 II=0 A20=0 SMM=0 HLT=0 ES =0000 00000000 00000000 00000000 CS =0000 00000000 00000000 00000000 SS =0000 00000000 00000000 00000000 DS =0000 00000000 00000000 00000000 FS =0000 00000000 00000000 00000000 GS =0000 00000000 00000000 00000000 LDT=0000 00000000 00000000 00000000 TR =0000 00000000 00000000 00000000 GDT= 00000000 00000000 IDT= 00000000 00000000 CR0=00000000 CR2=00000000 CR3=00000000 CR4=00000000 DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 DR6=0000000000000000 DR7=0000000000000000 CCS=00000000 CCD=00000000 CCO=DYNAMIC EFER=0000000000000000 FCW=0000 FSW=0000 [ST=0] FTW=ff MXCSR=00000000 FPR0=0000000000000000 0000 FPR1=0000000000000000 0000 FPR2=0000000000000000 0000 FPR3=0000000000000000 0000 FPR4=0000000000000000 0000 FPR5=0000000000000000 0000 FPR6=0000000000000000 0000 FPR7=0000000000000000 0000 XMM00=00000000000000000000000000000000 XMM01=00000000000000000000000000000000 XMM02=00000000000000000000000000000000 XMM03=00000000000000000000000000000000 XMM04=00000000000000000000000000000000 XMM05=00000000000000000000000000000000 XMM06=00000000000000000000000000000000 XMM07=00000000000000000000000000000000 CPU Reset (CPU 1) EAX=00000000 EBX=00000000 ECX=00000000 EDX=000206a1 ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000 EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0000 00000000 0000ffff 00009300 CS =f000 ffff0000 0000ffff 00009b00 SS =0000 00000000 0000ffff 00009300 DS =0000 00000000 0000ffff 00009300 FS =0000 00000000 0000ffff 00009300 GS =0000 00000000 0000ffff 00009300 LDT=0000 00000000 0000ffff 00008200 TR =0000 00000000 0000ffff 00008b00 GDT= 00000000 0000ffff IDT= 00000000 0000ffff CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000 DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 DR6=00000000ffff0ff0 DR7=0000000000000400 CCS=00000000 CCD=00000000 CCO=DYNAMIC EFER=0000000000000000 FCW=037f FSW=0000 [ST=0] FTW=00 MXCSR=00001f80 FPR0=0000000000000000 0000 FPR1=0000000000000000 0000 FPR2=0000000000000000 0000 FPR3=0000000000000000 0000 FPR4=0000000000000000 0000 FPR5=0000000000000000 0000 FPR6=0000000000000000 0000 FPR7=0000000000000000 0000 XMM00=00000000000000000000000000000000 XMM01=00000000000000000000000000000000 XMM02=00000000000000000000000000000000 XMM03=00000000000000000000000000000000 XMM04=00000000000000000000000000000000 XMM05=00000000000000000000000000000000 XMM06=00000000000000000000000000000000 XMM07=000 qemu: terminating on signal 15 from pid 168133 Nikolay Shirokovskiy (2): log: remove 1k limit for error messages log: drain log of exiting qemu process carefully src/logging/log_handler.c | 39 +++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_domain.c | 4 ++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_process.c | 2 ++ src/util/virerror.c | 9 ++++----- 5 files changed, 48 insertions(+), 7 deletions(-) -- 1.8.3.1

2 3

[libvirt] [PATCH v6 0/5] Add native TLS encrypted chardev TCP support
by John Ferlan 12 Sep '16

12 Sep '16

v5: http://www.redhat.com/archives/libvir-list/2016-August/msg00282.html Patches 1-5 from that series already pushed Patch 6 from that series is removed Patch 7 is untouched and is patch 3 in this series Patches 8-9 modified for new paradigm (patches 4-5 of this series) Patch 1 [NEW] From patch 6 review comment - provide a way to allow disabling the TLS for a specific guest and serial TCP chardev. A new test was added which shows that the xml2argv output does not add the TLS object within the same "frame" as the test that would add it. Patch 2 [NEW] Rather than use a <secret> object for the serial TCP chardev, create a qemu.conf option as a host wide setting. The patch implements a 'chardev_tls_x509_secret_uuid' value which will be used by patches 4-5 rather than the serial TCP specific setting. Patch 3 is unchanged Patches 4-5 were adjusted in order to use the chardevTCPtlsx509secretUUID rather than the chardev TCP specific 'chardev->source.data.tcp.seclookupdef' value sourced from a <secret>. John Ferlan (5): domain: Add optional 'disableTLS' attribute for TCP chardev conf: Introduce {default|chardev}_tls_x509_secret_uuid qemu: Introduce qemuDomainChardevPrivatePtr qemu: Add a secret object to/for a chardev tcp with secret qemu: Add the ability to hotplug a secret object for TCP chardev TLS docs/formatdomain.html.in | 20 +++ docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 51 ++++++-- src/conf/domain_conf.h | 5 +- src/libxl/libxl_domain.c | 2 +- src/lxc/lxc_native.c | 2 +- src/qemu/libvirtd_qemu.aug | 2 + src/qemu/qemu.conf | 24 ++++ src/qemu/qemu_command.c | 34 ++++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_conf.c | 22 ++++ src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 143 ++++++++++++++++++++- src/qemu/qemu_domain.h | 30 ++++- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 63 ++++++++- src/qemu/qemu_hotplug.h | 3 +- src/qemu/qemu_parse_command.c | 4 +- src/qemu/qemu_process.c | 6 +- src/qemu/test_libvirtd_qemu.aug.in | 2 + src/vz/vz_sdk.c | 2 +- src/xenconfig/xen_sxpr.c | 2 +- tests/qemuhotplugtest.c | 2 +- ...argv-serial-tcp-tlsx509-chardev-disableTLS.args | 30 +++++ ...2argv-serial-tcp-tlsx509-chardev-disableTLS.xml | 50 +++++++ ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 ++++++ ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++++++ tests/qemuxml2argvtest.c | 22 ++++ ...mlout-serial-tcp-tlsx509-chardev-disableTLS.xml | 1 + tests/qemuxml2xmltest.c | 1 + 30 files changed, 588 insertions(+), 34 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-disableTLS.xml -- 2.7.4

2 7

[libvirt] [PATCH 0/2] try harder to get dest qemu errors on migation
by Nikolay Shirokovskiy 12 Sep '16

12 Sep '16

Hi, all. In case migration fails due to destination qemu exits unexpectedly user recevies the qemu log in the error message. Unfortunately log is truncated and the most interesting part is missed (below is the example of such a log [1]). Actually for the most cases the first patch will be enough to fix the issue. Originally I thought the problem is qemu logging and reading the log are not in sync (which is true) so I tried to fix it as well in the second patch. [1] log example: CPU Reset (CPU 0) EAX=00000000 EBX=00000000 ECX=00000000 EDX=00000000 ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000 EIP=00000000 EFL=00000000 [-------] CPL=0 II=0 A20=0 SMM=0 HLT=0 ES =0000 00000000 00000000 00000000 CS =0000 00000000 00000000 00000000 SS =0000 00000000 00000000 00000000 DS =0000 00000000 00000000 00000000 FS =0000 00000000 00000000 00000000 GS =0000 00000000 00000000 00000000 LDT=0000 00000000 00000000 00000000 TR =0000 00000000 00000000 00000000 GDT= 00000000 00000000 IDT= 00000000 00000000 CR0=00000000 CR2=00000000 CR3=00000000 CR4=00000000 DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 DR6=0000000000000000 DR7=0000000000000000 CCS=00000000 CCD=00000000 CCO=DYNAMIC EFER=0000000000000000 FCW=0000 FSW=0000 [ST=0] FTW=ff MXCSR=00000000 FPR0=0000000000000000 0000 FPR1=0000000000000000 0000 FPR2=0000000000000000 0000 FPR3=0000000000000000 0000 FPR4=0000000000000000 0000 FPR5=0000000000000000 0000 FPR6=0000000000000000 0000 FPR7=0000000000000000 0000 XMM00=00000000000000000000000000000000 XMM01=00000000000000000000000000000000 XMM02=00000000000000000000000000000000 XMM03=00000000000000000000000000000000 XMM04=00000000000000000000000000000000 XMM05=00000000000000000000000000000000 XMM06=00000000000000000000000000000000 XMM07=00000000000000000000000000000000 CPU Reset (CPU 1) EAX=00000000 EBX=00000000 ECX=00000000 EDX=000206a1 ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000 EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0000 00000000 0000ffff 00009300 CS =f000 ffff0000 0000ffff 00009b00 SS =0000 00000000 0000ffff 00009300 DS =0000 00000000 0000ffff 00009300 FS =0000 00000000 0000ffff 00009300 GS =0000 00000000 0000ffff 00009300 LDT=0000 00000000 0000ffff 00008200 TR =0000 00000000 0000ffff 00008b00 GDT= 00000000 0000ffff IDT= 00000000 0000ffff CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000 DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 DR6=00000000ffff0ff0 DR7=0000000000000400 CCS=00000000 CCD=00000000 CCO=DYNAMIC EFER=0000000000000000 FCW=037f FSW=0000 [ST=0] FTW=00 MXCSR=00001f80 FPR0=0000000000000000 0000 FPR1=0000000000000000 0000 FPR2=0000000000000000 0000 FPR3=0000000000000000 0000 FPR4=0000000000000000 0000 FPR5=0000000000000000 0000 FPR6=0000000000000000 0000 FPR7=0000000000000000 0000 XMM00=00000000000000000000000000000000 XMM01=00000000000000000000000000000000 XMM02=00000000000000000000000000000000 XMM03=00000000000000000000000000000000 XMM04=00000000000000000000000000000000 XMM05=00000000000000000000000000000000 XMM06=00000000000000000000000000000000 XMM07=000 qemu: terminating on signal 15 from pid 168133 Nikolay Shirokovskiy (2): log: remove 1k limit for error messages log: drain log of exiting qemu process carefully src/logging/log_handler.c | 39 +++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_domain.c | 4 ++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_process.c | 2 ++ src/util/virerror.c | 9 ++++----- 5 files changed, 48 insertions(+), 7 deletions(-) -- 1.8.3.1

1 0

[libvirt] [gconfig v2 0/6] Add support for changing listen address
by Visarion Alexandru 12 Sep '16

12 Sep '16

From: Visarion Alexandru <viorel.visarion(a)gmail.com> libvirt-domain-graphics-listen defines an abstract class used to represent one listen node. libvirt-domain-graphics-liste-address defines a class that represents a listen node of type 'address', for the graphics device. We have to first remove the 'listen' attribute of the graphics device before adding child 'listen' nodes, to avoid inconsistencies check. I added a test for the new API which uses a SPICE graphics (I noticed that generally APIs aren't tested on both SPICE and VNC) Visarion Alexandru (6): libvirt-gconfig: Include GIO dependency Introduce libvirt-domain-graphics-listen Introduce libvirt-gconfig-domain-graphics-listen-address config: Add vnc listen getter/setter config: Add spice listen getter/setter tests: Add test for GVirConfigDomainGraphicsListenAddress configure.ac | 2 + libvirt-gconfig-1.0.pc.in | 2 +- libvirt-gconfig/Makefile.am | 10 +- ...ibvirt-gconfig-domain-graphics-listen-address.c | 128 +++++++++++++++++++++ ...ibvirt-gconfig-domain-graphics-listen-address.h | 78 +++++++++++++ .../libvirt-gconfig-domain-graphics-listen.c | 49 ++++++++ .../libvirt-gconfig-domain-graphics-listen.h | 65 +++++++++++ .../libvirt-gconfig-domain-graphics-spice.c | 84 ++++++++++++++ .../libvirt-gconfig-domain-graphics-spice.h | 4 + .../libvirt-gconfig-domain-graphics-vnc.c | 84 ++++++++++++++ .../libvirt-gconfig-domain-graphics-vnc.h | 4 + libvirt-gconfig/libvirt-gconfig.h | 2 + libvirt-gconfig/libvirt-gconfig.sym | 12 ++ tests/test-gconfig.c | 36 ++++++ .../xml/gconfig-domain-device-graphics-listen.xml | 7 ++ vapi/Makefile.am | 1 + 16 files changed, 565 insertions(+), 3 deletions(-) create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-listen-address.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-listen-address.h create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-listen.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-listen.h create mode 100644 tests/xml/gconfig-domain-device-graphics-listen.xml -- 2.7.4

3 14

[libvirt] [PATCH] util: Quiet the logging if perf file doesn't exist
by John Ferlan 12 Sep '16

12 Sep '16

Commit id 'b00d7f29' shifted the opening of the /sys/devices/intel_cqm/type file from event enable to perf event initialization. If the file did not exist, then an error would be written to the domain log: 2016-09-06 20:51:21.677+0000: 7310: error : virFileReadAll:1360 : Failed to open file '/sys/devices/intel_cqm/type': No such file or directory Since the error is now handled in virPerfEventEnable by checking if the event_attr->attrType == 0 for CMT, MBML, and MBMT events - we can just use the Quiet API in order to not log the error we're going to throw away. Additionally, rather than using virReportSystemError, use virReportError and VIR_ERR_ARGUMENT_UNSUPPORTED in order to signify that support isn't there for that type of perf event - adjust the error message as well. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/util/virperf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/virperf.c b/src/util/virperf.c index cd66b05..a65a4bf 100644 --- a/src/util/virperf.c +++ b/src/util/virperf.c @@ -116,7 +116,7 @@ virPerfRdtAttrInit(void) char *tmp = NULL; unsigned int attr_type = 0; - if (virFileReadAll("/sys/devices/intel_cqm/type", 10, &buf) < 0) + if (virFileReadAllQuiet("/sys/devices/intel_cqm/type", 10, &buf) < 0) goto error; if ((tmp = strchr(buf, '\n'))) @@ -174,9 +174,9 @@ virPerfEventEnable(virPerfPtr perf, if (event_attr->attrType == 0 && (type == VIR_PERF_EVENT_CMT || type == VIR_PERF_EVENT_MBMT || type == VIR_PERF_EVENT_MBML)) { - virReportSystemError(errno, - _("Unable to open perf event for %s"), - virPerfEventTypeToString(event->type)); + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("unable to enable perf event for %s"), + virPerfEventTypeToString(event->type)); return -1; } -- 2.7.4

2 1

[libvirt] [PATCH 0/2] qemu: hotplug: Fix two issues with drives made empty by using starup policy
by Peter Krempa 12 Sep '16

12 Sep '16

If a cdrom drive is emptied by 'requisite' startup policy the policy setting would not be cleared and due to a bug in qemu the cdrom image could not be changed. The first patch clears the startup policy setting and the second one fixes ejection of cdroms if --force is specified. Also I've reported the issue in qemu https://bugzilla.redhat.com/show_bug.cgi?id=1373264 Peter Krempa (2): qemu: domain: Clear startup policy for dropped removable media qemu: hotplug: Don't wait if cdrom tray is opened forcibly src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_hotplug.c | 9 ++++----- 2 files changed, 6 insertions(+), 5 deletions(-) -- 2.9.2

2 3

[libvirt] [PATCH v2 00/10] Introduce NVDIMM support
by Michal Privoznik 09 Sep '16

09 Sep '16

This is v2 of: https://www.redhat.com/archives/libvir-list/2016-August/msg00055.html diff to v1: - Rebase to the latest HEAD - Introduced relabeling for host side of NVDIMM Michal Privoznik (10): Introduce NVDIMM memory model qemu: Introduce QEMU_CAPS_DEVICE_NVDIMM qemu: Implement NVDIMM conf: Introduce memAccess to <memory/> qemu: Implement memAccess for <memory/> banks security_dac: Label host side of NVDIMM security_selinux: Label host side of NVDIMM security: Introduce internal APIs for memdev labelling secdrivers: Implement memdev relabel APIs qemu_hotplug: Relabel memdev docs/formatdomain.html.in | 41 ++++++-- docs/schemas/domaincommon.rng | 51 ++++++---- src/conf/domain_conf.c | 112 ++++++++++++++++----- src/conf/domain_conf.h | 4 + src/libvirt_private.syms | 4 + src/qemu/qemu_alias.c | 12 ++- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 87 +++++++++++----- src/qemu/qemu_command.h | 2 + src/qemu/qemu_domain.c | 29 ++++-- src/qemu/qemu_hotplug.c | 17 +++- src/security/security_dac.c | 76 ++++++++++++++ src/security/security_driver.h | 9 ++ src/security/security_manager.c | 56 +++++++++++ src/security/security_manager.h | 7 ++ src/security/security_nop.c | 19 ++++ src/security/security_selinux.c | 69 +++++++++++++ src/security/security_stack.c | 38 +++++++ tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + .../qemuxml2argv-hugepages-numa.args | 5 +- .../qemuxml2argv-hugepages-pages.args | 24 ++--- .../qemuxml2argv-hugepages-pages2.args | 8 +- .../qemuxml2argv-hugepages-pages3.args | 4 +- .../qemuxml2argv-hugepages-shared.args | 22 ++-- .../qemuxml2argv-memory-hotplug-dimm-addr.args | 5 +- .../qemuxml2argv-memory-hotplug-dimm.args | 5 +- ...muxml2argv-memory-hotplug-nvdimm-memAccess.args | 26 +++++ ...emuxml2argv-memory-hotplug-nvdimm-memAccess.xml | 49 +++++++++ .../qemuxml2argv-memory-hotplug-nvdimm.args | 25 +++++ .../qemuxml2argv-memory-hotplug-nvdimm.xml | 49 +++++++++ tests/qemuxml2argvtest.c | 6 +- 33 files changed, 743 insertions(+), 123 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-memAccess.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-memAccess.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm.xml -- 2.8.4

4 22

[libvirt] [PATCH v5 0/9] Add native TLS encrypted chardev TCP support
by John Ferlan 09 Sep '16

09 Sep '16

v4: http://www.redhat.com/archives/libvir-list/2016-June/msg01709.html Since I have it on a branch and have been updating, I figured I'd post the most recent stuff. Patches 1-4 were "partially" ACK'd in v2 of this series, but there's been changes to the conf handling upstream. Patch 5 adds a new secret type 'tls'. Previous incarnations of these changes borrowed a common secret type, but this one is specific. It's more or less what got removed for LUKS with the names changed to protect the innocent (reference Dragnet). Patches 6-9 is what was mostly missing in the earlier series. Differences to v4... mostly updates/merges with the numerous changes to master since that time. I lost track of whether the desire was to have /etc/pki/libvirt-%s or /etc/pki/qemu-%s directories... I think we've been using the libvirt-%s for vnc/spice so far, so I just followed that for chardev although there was a comment at one time to use qemu-chardev during review of patch 2 of the v2 series: http://www.redhat.com/archives/libvir-list/2016-June/msg01072.html John Ferlan (9): conf: Add new default TLS X.509 certificate default directory conf: Introduce chartcp_tls_x509_cert_dir qemu: Add support for TLS X.509 path to TCP chardev backend qemu: Add the ability to hotplug the TLS X.509 environment conf: Add new secret type "tls" conf: Add new secret element for tcp chardev qemu: Introduce qemuDomainChardevPrivatePtr qemu: Add a secret object to/for a chardev tcp with secret qemu: Add the ability to hotplug a secret object for TCP chardev TLS docs/aclpolkit.html.in | 4 + docs/formatdomain.html.in | 29 +++++ docs/formatsecret.html.in | 59 ++++++++- docs/schemas/domaincommon.rng | 21 +++ docs/schemas/secret.rng | 10 ++ include/libvirt/libvirt-secret.h | 1 + src/access/viraccessdriverpolkit.c | 13 ++ src/conf/domain_conf.c | 64 ++++++++-- src/conf/domain_conf.h | 8 +- src/conf/secret_conf.c | 23 +++- src/conf/secret_conf.h | 1 + src/conf/virsecretobj.c | 5 + src/libxl/libxl_domain.c | 2 +- src/lxc/lxc_native.c | 2 +- src/qemu/libvirtd_qemu.aug | 11 +- src/qemu/qemu.conf | 83 +++++++++--- src/qemu/qemu_alias.c | 16 +++ src/qemu/qemu_alias.h | 3 + src/qemu/qemu_command.c | 141 ++++++++++++++++++++- src/qemu/qemu_command.h | 9 ++ src/qemu/qemu_conf.c | 57 ++++++++- src/qemu/qemu_conf.h | 7 + src/qemu/qemu_domain.c | 124 +++++++++++++++++- src/qemu/qemu_domain.h | 22 ++++ src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 104 ++++++++++++++- src/qemu/qemu_hotplug.h | 3 +- src/qemu/qemu_monitor_json.c | 9 ++ src/qemu/qemu_parse_command.c | 4 +- src/qemu/qemu_process.c | 2 +- src/qemu/test_libvirtd_qemu.aug.in | 5 + src/vz/vz_sdk.c | 2 +- src/xenconfig/xen_sxpr.c | 2 +- tests/qemuhotplugtest.c | 2 +- .../qemuxml2argv-serial-tcp-tlsx509-chardev.args | 33 +++++ .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 41 ++++++ ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 ++++++ ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 51 ++++++++ tests/qemuxml2argvtest.c | 21 +++ .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 50 ++++++++ ...ml2xmlout-serial-tcp-tlsx509-secret-chardev.xml | 1 + tests/qemuxml2xmltest.c | 2 + tests/secretxml2xmlin/usage-tls.xml | 7 + tests/secretxml2xmltest.c | 1 + 44 files changed, 1038 insertions(+), 57 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-secret-chardev.xml create mode 100644 tests/secretxml2xmlin/usage-tls.xml -- 2.7.4

2 18

Re: [libvirt] [PATCH v7 0/4] Add Mediated device support
by Alex Williamson 08 Sep '16

08 Sep '16

Hi folks, At KVM Forum we had a BoF session primarily around the mediated device sysfs interface. I'd like to share what I think we agreed on and the "problem areas" that still need some work so we can get the thoughts and ideas from those who weren't able to attend. DanPB expressed some concern about the mdev_supported_types sysfs interface, which exposes a flat csv file with fields like "type", "number of instance", "vendor string", and then a bunch of type specific fields like "framebuffer size", "resolution", "frame rate limit", etc. This is not entirely machine parsing friendly and sort of abuses the sysfs concept of one value per file. Example output taken from Neo's libvirt RFC: cat /sys/bus/pci/devices/0000:86:00.0/mdev_supported_types # vgpu_type_id, vgpu_type, max_instance, num_heads, frl_config, framebuffer, max_resolution 11 ,"GRID M60-0B", 16, 2, 45, 512M, 2560x1600 12 ,"GRID M60-0Q", 16, 2, 60, 512M, 2560x1600 13 ,"GRID M60-1B", 8, 2, 45, 1024M, 2560x1600 14 ,"GRID M60-1Q", 8, 2, 60, 1024M, 2560x1600 15 ,"GRID M60-2B", 4, 2, 45, 2048M, 2560x1600 16 ,"GRID M60-2Q", 4, 4, 60, 2048M, 2560x1600 17 ,"GRID M60-4Q", 2, 4, 60, 4096M, 3840x2160 18 ,"GRID M60-8Q", 1, 4, 60, 8192M, 3840x2160 The create/destroy then looks like this: echo "$mdev_UUID:vendor_specific_argument_list" > /sys/bus/pci/devices/.../mdev_create echo "$mdev_UUID:vendor_specific_argument_list" > /sys/bus/pci/devices/.../mdev_destroy "vendor_specific_argument_list" is nebulous. So the idea to fix this is to explode this into a directory structure, something like: ├── mdev_destroy └── mdev_supported_types ├── 11 │ ├── create │ ├── description │ └── max_instances ├── 12 │ ├── create │ ├── description │ └── max_instances └── 13 ├── create ├── description └── max_instances Note that I'm only exposing the minimal attributes here for simplicity, the other attributes would be included in separate files and we would require vendors to create standard attributes for common device classes. For vGPUs like NVIDIA where we don't support multiple types concurrently, this directory structure would update as mdev devices are created, removing no longer available types. I carried forward max_instances here, but perhaps we really want to copy SR-IOV and report a max and current allocation. Creation and deletion is simplified as we can simply "echo $UUID > create" per type. I don't understand why destroy had a parameter list, so here I imagine we can simply do the same... in fact, I'd actually rather see a "remove" sysfs entry under each mdev device, so we remove it at the device rather than in some central location (any objections?). We discussed how this might look with Intel devices which do allow mixed vGPU types concurrently. We believe, but need confirmation, that the vendor driver could still make a finite set of supported types, perhaps with additional module options to the vendor driver to enable more "exotic" types. So for instance if IGD vGPUs are based on power-of-2 portions of the framebuffer size, then the vendor driver could list types with 32MB, 64MB, 128MB, etc in useful and popular sizes. As vGPUs are allocated, the larger sizes may become unavailable. We still don't have any way for the admin to learn in advance how the available supported types will change once mdev devices start to be created. I'm not sure how we can create a specification for this, so probing by creating devices may be the most flexible model. The other issue is the start/stop requirement, which was revealed to setup peer-to-peer resources between vGPUs which is a limited hardware resource. We'd really like to have these happen automatically on the first open of a vfio mdev device file and final release. So we brainstormed how the open/release callbacks could know the other mdev devices for a given user. This is where the instance number came into play previously. This is an area that needs work. There was a thought that perhaps on open() the vendor driver could look at the user pid and use that to associate with other devices, but the problem here is that we open and begin access to each device, so devices do this discovery serially rather than in parallel as desired. (we might not fault in mmio space yet though, so I wonder if open() could set the association of mdev to pid, then the first mmio fault would trigger the resource allocation? Then all the "magic" would live in the vendor driver. open() could fail if the pid already has running mdev devices and the vendor driver chooses not to support hotplug) One comment was that for a GPU that only supports homogeneous vGPUs, libvirt may choose to create all the vGPUs in advance and handle them as we do SR-IOV VFs. The UUID+instance model would preclude such a use case. We also considered whether iommu groups could be (ab)used for this use case, peer-to-peer would in fact be an iommu grouping constraint afterall. This would have the same UUID+instance constraint as above though and would require some sort of sysfs interface for the user to be able to create multiple mdevs within a group. Everyone was given homework to think about this on their flights home, so I expect plenty of ideas by now ;) Overall I think mediated devices were well received by the community, so let's keep up the development and discussion to bring it to fruition. Thanks, Alex

11 48

[libvirt] [PATCH] usb: allow host devices to be specified by port
by Thomas Hebb 08 Sep '16

08 Sep '16

Previously, only VID/PID and bus/device matching were supported. Neither of these provide a stable and persistent way of assigning a guest a specific host device out of several with the same VID and PID, as device numbers change on every enumeration. Add a third method of matching, bus/port, which allows a specific port on the host to be specified using the dotted notation found in Linux's "devpath" sysfs attribute. --- src/conf/domain_conf.c | 36 ++++- src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 2 - src/util/virhostdev.c | 126 ++++++++-------- src/util/virusb.c | 160 ++++++++------------- src/util/virusb.h | 23 ++- tests/virusbtest.c | 124 ++++++++++------ .../sys_bus_usb/devices/1-1.5.3.1/devpath | 1 + .../sys_bus_usb/devices/1-1.5.3.3/devpath | 1 + .../sys_bus_usb/devices/1-1.5.3/devpath | 1 + .../sys_bus_usb/devices/1-1.5.4/devpath | 1 + .../sys_bus_usb/devices/1-1.5.5/devpath | 1 + .../sys_bus_usb/devices/1-1.5.6/devpath | 1 + .../sys_bus_usb/devices/1-1.5/devpath | 1 + .../sys_bus_usb/devices/1-1.6/devpath | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/devpath | 1 + .../sys_bus_usb/devices/2-1.2/devpath | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/devpath | 1 + .../sys_bus_usb/devices/usb1/devpath | 1 + .../sys_bus_usb/devices/usb2/devpath | 1 + .../sys_bus_usb/devices/usb3/devpath | 1 + .../sys_bus_usb/devices/usb4/devpath | 1 + 22 files changed, 266 insertions(+), 222 deletions(-) create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9f7b906..a587cc2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5531,7 +5531,8 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, goto out; } } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { - char *bus, *device; + char *bus, *device, *port; + bool dev_or_port = false; bus = virXMLPropString(cur, "bus"); if (bus) { @@ -5557,10 +5558,24 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, VIR_FREE(device); goto out; } + dev_or_port = true; VIR_FREE(device); - } else { + } + + port = virXMLPropString(cur, "port"); + if (port) { + if (*port) { + usbsrc->port = port; + dev_or_port = true; + } else { + VIR_FREE(port); + } + } + + if (!dev_or_port) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("usb address needs device id")); + _("usb address needs either device id " + "or port")); goto out; } } else { @@ -20326,10 +20341,17 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n", usbsrc->vendor); virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", usbsrc->product); } - if (usbsrc->bus || usbsrc->device) { - virBufferAsprintf(buf, "<address %sbus='%d' device='%d'/>\n", - includeTypeInAddr ? "type='usb' " : "", - usbsrc->bus, usbsrc->device); + if (usbsrc->bus || usbsrc->device || usbsrc->port) { + virBufferAddLit(buf, "<address"); + if (includeTypeInAddr) + virBufferAddLit(buf, " type='usb'"); + if (usbsrc->bus) + virBufferAsprintf(buf, " bus='%u'", usbsrc->bus); + if (usbsrc->device) + virBufferAsprintf(buf, " device='%u'", usbsrc->device); + if (usbsrc->port) + virBufferAsprintf(buf, " port='%s'", usbsrc->port); + virBufferAddLit(buf, "/>\n"); } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ba0ad5f..91bd621 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -317,6 +317,8 @@ VIR_ENUM_DECL(virDomainHostdevSubsysSCSIProtocol) typedef struct _virDomainHostdevSubsysUSB virDomainHostdevSubsysUSB; typedef virDomainHostdevSubsysUSB *virDomainHostdevSubsysUSBPtr; struct _virDomainHostdevSubsysUSB { + char *port; + bool autoAddress; /* bus/device were filled automatically based on vendor/product */ unsigned bus; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ccb4c5e..d412eac 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2511,8 +2511,6 @@ virURIResolveAlias; # util/virusb.h virUSBDeviceFileIterate; virUSBDeviceFind; -virUSBDeviceFindByBus; -virUSBDeviceFindByVendor; virUSBDeviceFree; virUSBDeviceGetBus; virUSBDeviceGetDevno; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9b5ca6f..e809c97 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1179,89 +1179,95 @@ virHostdevMarkUSBDevices(virHostdevManagerPtr mgr, static int +virHostdevFindUSBDeviceWithFlags(virDomainHostdevDefPtr hostdev, + bool mandatory, + unsigned int flags, + virUSBDevicePtr *usb) +{ + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + unsigned vendor = usbsrc->vendor; + unsigned product = usbsrc->product; + unsigned bus = usbsrc->bus; + char *port = usbsrc->port; + unsigned device = usbsrc->device; + virUSBDeviceListPtr devs; + int rc; + + rc = virUSBDeviceFind(vendor, product, bus, device, port, NULL, + mandatory, flags, &devs); + if (rc < 0) + return -1; + + if (rc == 1) { + *usb = virUSBDeviceListGet(devs, 0); + virUSBDeviceListSteal(devs, *usb); + } + virObjectUnref(devs); + + if (rc > 1) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Multiple USB devices for %x:%x, " + "use <address> to specify one"), + vendor, product); + return -1; + } + + return rc; +} + +static int virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, bool mandatory, virUSBDevicePtr *usb) { virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; unsigned vendor = usbsrc->vendor; - unsigned product = usbsrc->product; unsigned bus = usbsrc->bus; unsigned device = usbsrc->device; + char *port = usbsrc->port; bool autoAddress = usbsrc->autoAddress; + unsigned int flags = 0; int rc; *usb = NULL; - if (vendor && bus) { - rc = virUSBDeviceFind(vendor, product, bus, device, - NULL, - autoAddress ? false : mandatory, - usb); - if (rc < 0) { - return -1; - } else if (!autoAddress) { - goto out; - } else { - VIR_INFO("USB device %x:%x could not be found at previous" - " address (bus:%u device:%u)", - vendor, product, bus, device); - } - } + /* First attempt, matching on all known fields. */ + if (vendor) + flags |= USB_DEVICE_FIND_BY_VENDOR; + if (device) + flags |= USB_DEVICE_FIND_BY_DEVICE; + if (port) + flags |= USB_DEVICE_FIND_BY_PORT; + + rc = virHostdevFindUSBDeviceWithFlags(hostdev, + autoAddress ? false : mandatory, + flags, usb); + if (rc < 0) + return -1; - /* When vendor is specified, its USB address is either unspecified or the - * device could not be found at the USB device where it had been - * automatically found before. - */ - if (vendor) { - virUSBDeviceListPtr devs; + if (rc != 1 && autoAddress) { + VIR_INFO("USB device could not be found at previous address " + "(bus:%u device:%u)", bus, device); - rc = virUSBDeviceFindByVendor(vendor, product, NULL, mandatory, &devs); - if (rc < 0) - return -1; + /* Second attempt, for when the device number has changed. */ + flags &= ~((unsigned int) USB_DEVICE_FIND_BY_DEVICE); + usbsrc->device = 0; - if (rc == 1) { - *usb = virUSBDeviceListGet(devs, 0); - virUSBDeviceListSteal(devs, *usb); - } - virObjectUnref(devs); - - if (rc == 0) { - goto out; - } else if (rc > 1) { - if (autoAddress) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Multiple USB devices for %x:%x were found," - " but none of them is at bus:%u device:%u"), - vendor, product, bus, device); - } else { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Multiple USB devices for %x:%x, " - "use <address> to specify one"), - vendor, product); - } + rc = virHostdevFindUSBDeviceWithFlags(hostdev, mandatory, + flags, usb); + + if (rc < 0) return -1; - } + } + if (!*usb) { + hostdev->missing = true; + } else if (!usbsrc->bus || !usbsrc->device) { usbsrc->bus = virUSBDeviceGetBus(*usb); usbsrc->device = virUSBDeviceGetDevno(*usb); usbsrc->autoAddress = true; - - if (autoAddress) { - VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved" - " from bus:%u device:%u)", - vendor, product, - usbsrc->bus, usbsrc->device, - bus, device); - } - } else if (!vendor && bus) { - if (virUSBDeviceFindByBus(bus, device, NULL, mandatory, usb) < 0) - return -1; } - out: - if (!*usb) - hostdev->missing = true; return 0; } diff --git a/src/util/virusb.c b/src/util/virusb.c index 6a001a7..2e15849 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -68,12 +68,6 @@ struct _virUSBDeviceList { virUSBDevicePtr *devs; }; -typedef enum { - USB_DEVICE_ALL = 0, - USB_DEVICE_FIND_BY_VENDOR = 1 << 0, - USB_DEVICE_FIND_BY_BUS = 1 << 1, -} virUSBDeviceFindFlags; - static virClassPtr virUSBDeviceListClass; static void virUSBDeviceListDispose(void *obj); @@ -119,11 +113,33 @@ static int virUSBSysReadFile(const char *f_name, const char *d_name, return ret; } +static int virUSBSysReadFileStr(const char *f_name, const char *d_name, + char **value) +{ + int ret = -1, tmp; + char *buf = NULL; + char *filename = NULL; + + tmp = virAsprintf(&filename, USB_SYSFS "/devices/%s/%s", d_name, f_name); + if (tmp < 0) + goto cleanup; + + if (virFileReadAll(filename, 1024, &buf) < 0) + goto cleanup; + + *value = buf; + ret = 0; + cleanup: + VIR_FREE(filename); + return ret; +} + static virUSBDeviceListPtr virUSBDeviceSearch(unsigned int vendor, unsigned int product, unsigned int bus, unsigned int devno, + const char *port, const char *vroot, unsigned int flags) { @@ -143,6 +159,8 @@ virUSBDeviceSearch(unsigned int vendor, while ((direrr = virDirRead(dir, &de, USB_SYSFS "/devices")) > 0) { unsigned int found_prod, found_vend, found_bus, found_devno; + char *found_port; + bool port_matches; char *tmpstr = de->d_name; if (strchr(de->d_name, ':')) @@ -170,16 +188,31 @@ virUSBDeviceSearch(unsigned int vendor, 10, &found_devno) < 0) goto cleanup; + if (virUSBSysReadFileStr("devpath", de->d_name, + &found_port) < 0) { + goto cleanup; + } else { + found_port[strlen(found_port) - 1] = '\0'; /* remove newline */ + port_matches = STREQ_NULLABLE(found_port, port); + VIR_FREE(found_port); + } + if ((flags & USB_DEVICE_FIND_BY_VENDOR) && (found_prod != product || found_vend != vendor)) continue; - if (flags & USB_DEVICE_FIND_BY_BUS) { + if (flags & USB_DEVICE_FIND_BY_DEVICE) { if (found_bus != bus || found_devno != devno) continue; found = true; } + if (flags & USB_DEVICE_FIND_BY_PORT) { + if (found_bus != bus || !port_matches) + continue; + found = true; + } + usb = virUSBDeviceNew(found_bus, found_devno, vroot); if (!usb) goto cleanup; @@ -204,36 +237,43 @@ virUSBDeviceSearch(unsigned int vendor, } int -virUSBDeviceFindByVendor(unsigned int vendor, - unsigned int product, - const char *vroot, - bool mandatory, - virUSBDeviceListPtr *devices) +virUSBDeviceFind(unsigned int vendor, + unsigned int product, + unsigned int bus, + unsigned int devno, + const char *port, + const char *vroot, + bool mandatory, + unsigned int flags, + virUSBDeviceListPtr *devices) { virUSBDeviceListPtr list; int count; - if (!(list = virUSBDeviceSearch(vendor, product, 0, 0, - vroot, - USB_DEVICE_FIND_BY_VENDOR))) + if (!(list = virUSBDeviceSearch(vendor, product, bus, devno, port, + vroot, flags))) return -1; - if (list->count == 0) { + count = list->count; + if (count == 0) { virObjectUnref(list); if (!mandatory) { - VIR_DEBUG("Did not find USB device %x:%x", - vendor, product); if (devices) *devices = NULL; return 0; } virReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device %x:%x"), vendor, product); + _("Did not find a matching USB device (matching " + "on%s%s%s all others ignored): vid:%04x, pid:%04x, " + "bus:%u, device:%u, port:%s"), + flags & USB_DEVICE_FIND_BY_VENDOR ? " vid/pid," : "", + flags & USB_DEVICE_FIND_BY_DEVICE ? " bus/device," : "", + flags & USB_DEVICE_FIND_BY_PORT ? " bus/port," : "", + vendor, product, bus, devno, port ? port : "(null)"); return -1; } - count = list->count; if (devices) *devices = list; else @@ -242,86 +282,6 @@ virUSBDeviceFindByVendor(unsigned int vendor, return count; } -int -virUSBDeviceFindByBus(unsigned int bus, - unsigned int devno, - const char *vroot, - bool mandatory, - virUSBDevicePtr *usb) -{ - virUSBDeviceListPtr list; - - if (!(list = virUSBDeviceSearch(0, 0, bus, devno, - vroot, - USB_DEVICE_FIND_BY_BUS))) - return -1; - - if (list->count == 0) { - virObjectUnref(list); - if (!mandatory) { - VIR_DEBUG("Did not find USB device bus:%u device:%u", - bus, devno); - if (usb) - *usb = NULL; - return 0; - } - - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device bus:%u device:%u"), - bus, devno); - return -1; - } - - if (usb) { - *usb = virUSBDeviceListGet(list, 0); - virUSBDeviceListSteal(list, *usb); - } - virObjectUnref(list); - - return 0; -} - -int -virUSBDeviceFind(unsigned int vendor, - unsigned int product, - unsigned int bus, - unsigned int devno, - const char *vroot, - bool mandatory, - virUSBDevicePtr *usb) -{ - virUSBDeviceListPtr list; - - unsigned int flags = USB_DEVICE_FIND_BY_VENDOR|USB_DEVICE_FIND_BY_BUS; - if (!(list = virUSBDeviceSearch(vendor, product, bus, devno, - vroot, flags))) - return -1; - - if (list->count == 0) { - virObjectUnref(list); - if (!mandatory) { - VIR_DEBUG("Did not find USB device %x:%x bus:%u device:%u", - vendor, product, bus, devno); - if (usb) - *usb = NULL; - return 0; - } - - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device %x:%x bus:%u device:%u"), - vendor, product, bus, devno); - return -1; - } - - if (usb) { - *usb = virUSBDeviceListGet(list, 0); - virUSBDeviceListSteal(list, *usb); - } - virObjectUnref(list); - - return 0; -} - virUSBDevicePtr virUSBDeviceNew(unsigned int bus, unsigned int devno, diff --git a/src/util/virusb.h b/src/util/virusb.h index f98ea21..0834cd4 100644 --- a/src/util/virusb.h +++ b/src/util/virusb.h @@ -35,29 +35,26 @@ typedef virUSBDevice *virUSBDevicePtr; typedef struct _virUSBDeviceList virUSBDeviceList; typedef virUSBDeviceList *virUSBDeviceListPtr; +typedef enum { + USB_DEVICE_ALL = 0, + USB_DEVICE_FIND_BY_VENDOR = 1 << 0, + USB_DEVICE_FIND_BY_DEVICE = 1 << 1, + USB_DEVICE_FIND_BY_PORT = 1 << 2, +} virUSBDeviceFindFlags; + virUSBDevicePtr virUSBDeviceNew(unsigned int bus, unsigned int devno, const char *vroot); -int virUSBDeviceFindByBus(unsigned int bus, - unsigned int devno, - const char *vroot, - bool mandatory, - virUSBDevicePtr *usb); - -int virUSBDeviceFindByVendor(unsigned int vendor, - unsigned int product, - const char *vroot, - bool mandatory, - virUSBDeviceListPtr *devices); - int virUSBDeviceFind(unsigned int vendor, unsigned int product, unsigned int bus, unsigned int devno, + const char *port, const char *vroot, bool mandatory, - virUSBDevicePtr *usb); + unsigned int flags, + virUSBDeviceListPtr *devices); void virUSBDeviceFree(virUSBDevicePtr dev); int virUSBDeviceSetUsedBy(virUSBDevicePtr dev, diff --git a/tests/virusbtest.c b/tests/virusbtest.c index 4bbfe4a..0782b73 100644 --- a/tests/virusbtest.c +++ b/tests/virusbtest.c @@ -33,7 +33,8 @@ typedef enum { FIND_BY_ALL, FIND_BY_VENDOR, - FIND_BY_BUS + FIND_BY_DEVICE, + FIND_BY_PORT } testUSBFindFlags; struct findTestInfo { @@ -42,6 +43,7 @@ struct findTestInfo { unsigned int product; unsigned int bus; unsigned int devno; + const char *port; const char *vroot; bool mandatory; int how; @@ -78,25 +80,31 @@ static int testDeviceFind(const void *opaque) virUSBDeviceListPtr devs = NULL; int rv = 0; size_t i, ndevs = 0; + unsigned int flags = 0; switch (info->how) { case FIND_BY_ALL: - rv = virUSBDeviceFind(info->vendor, info->product, - info->bus, info->devno, - info->vroot, info->mandatory, &dev); + flags = USB_DEVICE_FIND_BY_VENDOR | + USB_DEVICE_FIND_BY_DEVICE | + USB_DEVICE_FIND_BY_PORT; break; case FIND_BY_VENDOR: - rv = virUSBDeviceFindByVendor(info->vendor, info->product, - info->vroot, info->mandatory, &devs); + flags = USB_DEVICE_FIND_BY_VENDOR; break; - case FIND_BY_BUS: - rv = virUSBDeviceFindByBus(info->bus, info->devno, - info->vroot, info->mandatory, &dev); + case FIND_BY_DEVICE: + flags = USB_DEVICE_FIND_BY_DEVICE; + break; + case FIND_BY_PORT: + flags = USB_DEVICE_FIND_BY_PORT; break; } + rv = virUSBDeviceFind(info->vendor, info->product, + info->bus, info->devno, info->port, + info->vroot, info->mandatory, flags, &devs); + if (info->expectFailure) { - if (rv == 0) { + if (rv >= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "unexpected success"); } else { @@ -107,9 +115,18 @@ static int testDeviceFind(const void *opaque) goto cleanup; } + if (info->how != FIND_BY_VENDOR) { + if (rv == 1) { + dev = virUSBDeviceListGet(devs, 0); + virUSBDeviceListSteal(devs, dev); + } else { + goto cleanup; + } + } + switch (info->how) { case FIND_BY_ALL: - case FIND_BY_BUS: + case FIND_BY_DEVICE: if (virUSBDeviceFileIterate(dev, testDeviceFileActor, NULL) < 0) goto cleanup; break; @@ -155,14 +172,17 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) virUSBDeviceListPtr list = NULL; virUSBDeviceListPtr devlist = NULL; virUSBDevicePtr dev = NULL; + virUSBDeviceListPtr devs = NULL; int ret = -1; + int rv; size_t i, ndevs; if (!(list = virUSBDeviceListNew())) goto cleanup; #define EXPECTED_NDEVS_ONE 3 - if (virUSBDeviceFindByVendor(0x1d6b, 0x0002, NULL, true, &devlist) < 0) + if (virUSBDeviceFind(0x1d6b, 0x0002, 0, 0, NULL, NULL, true, + USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0) goto cleanup; ndevs = virUSBDeviceListCount(devlist); @@ -186,7 +206,8 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; #define EXPECTED_NDEVS_TWO 3 - if (virUSBDeviceFindByVendor(0x18d1, 0x4e22, NULL, true, &devlist) < 0) + if (virUSBDeviceFind(0x18d1, 0x4e22, 0, 0, NULL, NULL, true, + USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0) goto cleanup; ndevs = virUSBDeviceListCount(devlist); @@ -206,8 +227,17 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) EXPECTED_NDEVS_ONE + EXPECTED_NDEVS_TWO) < 0) goto cleanup; - if (virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, NULL, true, &dev) < 0) + rv = virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, "1.5.6", NULL, true, + USB_DEVICE_FIND_BY_VENDOR | + USB_DEVICE_FIND_BY_DEVICE | + USB_DEVICE_FIND_BY_PORT, &devs); + if (rv != 1) { goto cleanup; + } else { + dev = virUSBDeviceListGet(devs, 0); + virUSBDeviceListSteal(devs, dev); + } + virObjectUnref(devs); if (!virUSBDeviceListFind(list, dev)) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -239,49 +269,63 @@ mymain(void) { int rv = 0; -#define DO_TEST_FIND_FULL(name, vend, prod, bus, devno, vroot, mand, how, fail) \ - do { \ - struct findTestInfo data = { name, vend, prod, bus, \ - devno, vroot, mand, how, fail \ - }; \ - if (virTestRun("USBDeviceFind " name, testDeviceFind, &data) < 0) \ - rv = -1; \ +#define DO_TEST_FIND_FULL(name, vend, prod, bus, devno, \ + port, vroot, mand, how, fail) \ + do { \ + struct findTestInfo data = { name, vend, prod, bus, \ + devno, port, vroot, mand, how, fail \ + }; \ + if (virTestRun("USBDeviceFind " name, testDeviceFind, &data) < 0) \ + rv = -1; \ } while (0) -#define DO_TEST_FIND(name, vend, prod, bus, devno) \ - DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, true, \ +#define DO_TEST_FIND(name, vend, prod, bus, devno, port) \ + DO_TEST_FIND_FULL(name, vend, prod, bus, devno, port, NULL, true, \ FIND_BY_ALL, false) -#define DO_TEST_FIND_FAIL(name, vend, prod, bus, devno) \ - DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, true, \ +#define DO_TEST_FIND_FAIL(name, vend, prod, bus, devno, port) \ + DO_TEST_FIND_FULL(name, vend, prod, bus, devno, port, NULL, true, \ FIND_BY_ALL, true) -#define DO_TEST_FIND_BY_BUS(name, bus, devno) \ - DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \ - FIND_BY_BUS, false) -#define DO_TEST_FIND_BY_BUS_FAIL(name, bus, devno) \ - DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \ - FIND_BY_BUS, true) +#define DO_TEST_FIND_BY_DEVICE(name, bus, devno) \ + DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, NULL, true, \ + FIND_BY_DEVICE, false) +#define DO_TEST_FIND_BY_DEVICE_FAIL(name, bus, devno) \ + DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, NULL, true, \ + FIND_BY_DEVICE, true) #define DO_TEST_FIND_BY_VENDOR(name, vend, prod) \ - DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, true, \ + DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, NULL, true, \ FIND_BY_VENDOR, false) #define DO_TEST_FIND_BY_VENDOR_FAIL(name, vend, prod) \ - DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, true, \ + DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, NULL, true, \ FIND_BY_VENDOR, true) - DO_TEST_FIND("Nexus", 0x18d1, 0x4e22, 1, 20); - DO_TEST_FIND_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, 1, 25); - DO_TEST_FIND_FAIL("Bogus", 0xf00d, 0xbeef, 1024, 768); +#define DO_TEST_FIND_BY_PORT(name, bus, port) \ + DO_TEST_FIND_FULL(name, 101, 202, bus, 456, port, NULL, true, \ + FIND_BY_PORT, false) +#define DO_TEST_FIND_BY_PORT_FAIL(name, bus, port) \ + DO_TEST_FIND_FULL(name, 101, 202, bus, 456, port, NULL, true, \ + FIND_BY_PORT, true) + + DO_TEST_FIND("Nexus", 0x18d1, 0x4e22, 1, 20, "1.5.6"); + DO_TEST_FIND_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, 1, 25, "1.5.6"); + DO_TEST_FIND_FAIL("Nexus wrong port", 0x18d1, 0x4e22, 1, 25, "1.5.4"); + DO_TEST_FIND_FAIL("Bogus", 0xf00d, 0xbeef, 1024, 768, "1.2.3.4"); - DO_TEST_FIND_BY_BUS("integrated camera", 1, 5); - DO_TEST_FIND_BY_BUS_FAIL("wrong bus/devno combination", 2, 20); - DO_TEST_FIND_BY_BUS_FAIL("missing bus", 5, 20); - DO_TEST_FIND_BY_BUS_FAIL("missing devnum", 1, 158); + DO_TEST_FIND_BY_DEVICE("integrated camera", 1, 5); + DO_TEST_FIND_BY_DEVICE_FAIL("wrong bus/devno combination", 2, 20); + DO_TEST_FIND_BY_DEVICE_FAIL("missing bus", 5, 20); + DO_TEST_FIND_BY_DEVICE_FAIL("missing devnum", 1, 158); DO_TEST_FIND_BY_VENDOR("Nexus (multiple results)", 0x18d1, 0x4e22); DO_TEST_FIND_BY_VENDOR_FAIL("Bogus vendor and product", 0xf00d, 0xbeef); DO_TEST_FIND_BY_VENDOR_FAIL("Valid vendor", 0x1d6b, 0xbeef); + DO_TEST_FIND_BY_PORT("Logitech mouse", 1, "1.5.3.3"); + DO_TEST_FIND_BY_PORT_FAIL("wrong bus/port combination", 2, "1.5.3.3"); + DO_TEST_FIND_BY_PORT_FAIL("missing bus", 5, "1.5.3.3"); + DO_TEST_FIND_BY_PORT_FAIL("missing port", 1, "8.2.5"); + if (virTestRun("USB List test", testUSBList, NULL) < 0) rv = -1; diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath new file mode 100644 index 0000000..02a7fbe --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath @@ -0,0 +1 @@ +1.5.3.1 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath new file mode 100644 index 0000000..23ca863 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath @@ -0,0 +1 @@ +1.5.3.3 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath new file mode 100644 index 0000000..8af85be --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath @@ -0,0 +1 @@ +1.5.3 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath new file mode 100644 index 0000000..94fe62c --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath @@ -0,0 +1 @@ +1.5.4 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath new file mode 100644 index 0000000..9075be4 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath @@ -0,0 +1 @@ +1.5.5 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath new file mode 100644 index 0000000..eac1e0a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath @@ -0,0 +1 @@ +1.5.6 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath new file mode 100644 index 0000000..c239c60 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath @@ -0,0 +1 @@ +1.5 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath new file mode 100644 index 0000000..810ee4e --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath @@ -0,0 +1 @@ +1.6 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath @@ -0,0 +1 @@ +1 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath b/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath new file mode 100644 index 0000000..5625e59 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath @@ -0,0 +1 @@ +1.2 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath @@ -0,0 +1 @@ +1 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath @@ -0,0 +1 @@ +0 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath @@ -0,0 +1 @@ +0 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath @@ -0,0 +1 @@ +0 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath @@ -0,0 +1 @@ +0 -- 2.7.4

2 1

Re: [libvirt] [PATCH 00/16] Hyper-V: Improve 2008, Introduce 2012
by Fernando Casas Schössow 08 Sep '16

08 Sep '16

Hi there, I was wondering if any of these patches actually made it into libvirt code. I'm really looking forward hyper-v support improvement and features like shutdown VM (not destroy/turn off but clean shutdown) implemented. Especially interested in 2012/2012 R2 since 2008 is getting old and 2016 is right around the corner. I'm already using libvirt 2.1.0 with some patches I found in https://github.com/vikhyath/libvirt-hyperv-r2-2012 but it would be great to have the code reviewed and merged into libvirt releases. Thanks. Fer > The following patches include work originally done by Yves Vinter back > in 2014. The last patch introduces support for Hyper-V 2012, while > still > supporting 2008. I am not sure that the method I used to include the > 2012 > support is the best approach, mainly due to code duplication, but I am > open to suggestions on how to do this better. > > Jason Miesionczek (16): > hyperv: additional server 2008 wmi classes > hyperv: add cim types support to code generator > hyperv: add get capabilities > hyperv: implement connectGetVersion > hyperv: implement vcpu functions > hyperv: implement nodeGetFreeMemory > hyperv: implement ability to send xml soap requests > hyperv: introduce basic network driver > hyperv: add domain shutdown function > hyperv: add get scheduler functions > hyperv: add set memory functions > hyperv: set vpcu functions > hyperv: domain undefine functions > hyperv: domain define and associated functions > hyperv: network list functions > hyperv: introduce 2012 support > > src/Makefile.am | 2 + > src/hyperv/hyperv_driver.c | 1989 ++++++++++++++++++++++++++++++++- > src/hyperv/hyperv_driver_2012.c | 299 +++++ > src/hyperv/hyperv_driver_2012.h | 55 + > src/hyperv/hyperv_network_driver.c | 280 +++++ > src/hyperv/hyperv_network_driver.h | 30 + > src/hyperv/hyperv_private.h | 8 + > src/hyperv/hyperv_wmi.c | 709 +++++++++++- > src/hyperv/hyperv_wmi.h | 78 ++ > src/hyperv/hyperv_wmi_generator.input | 518 ++++++++- > src/hyperv/hyperv_wmi_generator.py | 68 +- > src/hyperv/openwsman.h | 4 + > 12 files changed, 3989 insertions(+), 51 deletions(-) > create mode 100644 src/hyperv/hyperv_driver_2012.c > create mode 100644 src/hyperv/hyperv_driver_2012.h > create mode 100644 src/hyperv/hyperv_network_driver.c > create mode 100644 src/hyperv/hyperv_network_driver.h

3 7

[libvirt] [PATCH] security: Fixing wrong label in virt-aa-helper.c.
by Julio Faracco 08 Sep '16

08 Sep '16

There is an issue with a wrong label inside vah_add_path(). The compilation fails with the error: make[3]: Entering directory '/tmp/libvirt/src' CC security/virt_aa_helper-virt-aa-helper.o security/virt-aa-helper.c: In function 'vah_add_path': security/virt-aa-helper.c:769:9: error: label 'clean' used but not defined goto clean; This patch moves 'clean' label to 'cleanup' label. Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com> --- src/security/virt-aa-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 2aa6e28..fc01076 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -766,7 +766,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi } if (VIR_STRDUP_QUIET(perms_new, perms) < 0) - goto clean; + goto cleanup; if (strchr(perms_new, 'w') != NULL) { readonly = false; -- 2.7.4

2 2

[libvirt] [PATCH 0/2] qemu: Remove stale transient def when migration fails
by Jiri Denemark 08 Sep '16

08 Sep '16

If a migration of a domain which is already defined on the destination host failed early (before we tried to start QEMU), we would forget to remove the incoming transient definition. Later on when someone starts the domain on the destination host, we will use the stale incoming definition and the persistent definition will just be ignored. https://bugzilla.redhat.com/show_bug.cgi?id=1368774 Jiri Denemark (2): Add helper for removing transient definition qemu: Remove stale transient def when migration fails src/conf/domain_conf.c | 19 +++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/libxl/libxl_domain.c | 8 +------- src/lxc/lxc_process.c | 7 +------ src/qemu/qemu_migration.c | 4 ++++ src/qemu/qemu_process.c | 7 +------ src/test/test_driver.c | 9 ++------- src/uml/uml_driver.c | 14 ++------------ 9 files changed, 32 insertions(+), 38 deletions(-) -- 2.10.0

2 4

[libvirt] Opening a read-only connection - with SASL auth.
by Yaniv Kaul 08 Sep '16

08 Sep '16

Looking at the collectd virt plugin, it opens a read only connection with libvirt[1]. We'd like to use it with SASL auth. (which oVirt configures libvirt to use by default, with a simple username/password) - is that possible? TIA, Y. [1] https://github.com/collectd/collectd/blob/d782a871aca61ac966ee8d928930719bd…

1 0

[libvirt] [PATCH] Allow address lease query with inactive domain
by Amador Pahim 08 Sep '16

08 Sep '16

The <drive>DomainInterfaceAddresses, when using the source type LEASE, does not need the domain to be running. It only checks the dhcp lease file and gets the address information from a valid lease, if any. This patch removes the virDomainObjIsActive(vm) check from the LEASE query type on both qemu and lxc drivers, keeping it only for query type AGENT on qemu driver (since lxc does not support AGENT type). Signed-off-by: Amador Pahim <amador(a)pahim.org> --- src/libxl/libxl_driver.c | 6 ------ src/qemu/qemu_driver.c | 12 ++++++------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 480fef9..9f8c36a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -6251,12 +6251,6 @@ libxlDomainInterfaceAddresses(virDomainPtr dom, if (virDomainInterfaceAddressesEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain is not running")); - goto cleanup; - } - switch (source) { case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE: ret = libxlGetDHCPInterfaces(dom, vm, ifaces); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 807e06d..a8dfa2f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19600,18 +19600,18 @@ qemuDomainInterfaceAddresses(virDomainPtr dom, if (virDomainInterfaceAddressesEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain is not running")); - goto cleanup; - } - switch (source) { case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE: ret = qemuGetDHCPInterfaces(dom, vm, ifaces); break; case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT: + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto cleanup; + } + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; -- 2.7.4

3 4

[libvirt] [PATCH] virt-admin: Document srv-list command in virt-admin's man page
by Erik Skultety 08 Sep '16

08 Sep '16

srv-list command was missing in the man page. Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- tools/virt-admin.pod | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index 2199038..c74707a 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -143,6 +143,20 @@ libvirtd:///system is used. =back +=head1 DAEMON COMMANDS + +Following commands allow to monitor daemon's state as well as directly change +its internal configuration. + +=over 4 + +=item B<srv-list> + +Lists all manageable servers on daemon the client is currently connected to +through administration interface. + +=back + =head1 SERVER COMMANDS Following commands manipulate daemon's server internal configuration. -- 2.5.5

2 2

[libvirt] [PATCH] virt-admin: Fix some English grammar in the manpage
by Erik Skultety 08 Sep '16

08 Sep '16

Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- tools/virt-admin.pod | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Pushed as trivial. diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index 0195001..020eb51 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -159,7 +159,7 @@ currently connected to. =head1 SERVER COMMANDS -Following commands manipulate daemon's server internal configuration. +The following commands manipulate daemon's server internal configuration. The I<server> is specified by its name. =over 4 @@ -281,7 +281,7 @@ I<--max-clients>. =head1 CLIENT COMMANDS -Following commands provide management and monitoring of clients connected to +The following commands provide management and monitoring of clients connected to one of daemon's available servers. Clients are specified by their numeric ID which is obtained by listing all clients connected to a specified server (see command B<srv-clients-list>). -- 2.5.5

1 0

[libvirt] [libvirt-php][PATCH 0/8] Couple of php fixes
by Michal Privoznik 08 Sep '16

08 Sep '16

Pushed under php-autopush rule. Michal Privoznik (8): libvirt_domain_get_network_info: Don't overwrite retval libvirt_storagepool_get_autostart: Rework translate_counter_type: Rework libvirt_domain_block_commit: Reindent libvirt_domain_qemu_agent_command: Reindent libvirt_domain_disk_add: Reindent libvirt_domain_disk_remove: Reindent libvirt_list_storagepools: Reindent src/libvirt-php.c | 129 ++++++++++++++++++++++++++---------------------------- 1 file changed, 61 insertions(+), 68 deletions(-) -- 2.8.4

1 8

[libvirt] [libvirt-php][PATCH] Don't leak @mime in libvirt_domain_get_screenshot_api
by Michal Privoznik 08 Sep '16

08 Sep '16

When getting a screenshot of a domain via virDomainScreenshot() the MIME type of a image is returned as a string which needs to be freed by caller. However, we are not doing that anywhere. ==24748== 48 bytes in 2 blocks are definitely lost in loss record 123 of 171 ==24748== at 0x4C29F80: malloc (vg_replace_malloc.c:296) ==24748== by 0x90674AB: xdr_string (xdr.c:790) ==24748== by 0xCD3AF1D: xdr_remote_nonnull_string (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0x9067B34: xdr_reference (xdr_ref.c:84) ==24748== by 0x9067C50: xdr_pointer (xdr_ref.c:135) ==24748== by 0xCD3B284: xdr_remote_string (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD3D288: xdr_remote_domain_screenshot_ret (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD5168E: virNetMessageDecodePayload (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD47E91: virNetClientProgramCall (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD1EBD3: callFull.isra.3 (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD29973: remoteDomainScreenshot (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCCDC24C: virDomainScreenshot (in /usr/lib64/libvirt.so.0.2003.0) Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Pushed under php-autopush rule. src/libvirt-php.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 8edcb10..57ef1a0 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -4767,8 +4767,10 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) } #ifndef EXTWIN - if (mkstemp(file) == 0) + if (mkstemp(file) == 0) { + free(mime); RETURN_FALSE; + } #endif if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0) { @@ -4776,6 +4778,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) (fd = open(file, O_WRONLY|O_TRUNC, 0666)) < 0) { virStreamFree(st); set_error_if_unset("Cannot get create file to save domain screenshot" TSRMLS_CC); + free(mime); RETURN_FALSE; } } @@ -4783,6 +4786,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) if (virStreamRecvAll(st, streamSink, &fd) < 0) { virStreamFree(st); set_error_if_unset("Cannot receive screenshot data" TSRMLS_CC); + free(mime); RETURN_FALSE; } @@ -4791,6 +4795,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) if (virStreamFinish(st) < 0) { virStreamFree(st); set_error_if_unset("Cannot close stream for domain" TSRMLS_CC); + free(mime); RETURN_FALSE; } @@ -4816,6 +4821,8 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) VIRT_ADD_ASSOC_STRING(return_value, "file", file); VIRT_ADD_ASSOC_STRING(return_value, "mime", mime); } + + free(mime); } /* -- 2.8.4

1 0

[libvirt] [PATCH] Avoid segfault in virt-aa-helper when handling read-only mount filesystems
by rufo 07 Sep '16

07 Sep '16

This patch fixes a segfault in virt-aa-helper caused by attempting to modify a string literal in situ. It is triggered when a domain has a <filesystem> with type='mount' configured readonly, and libvirt is using the AppArmor security driver for sVirt confinement. --- src/security/virt-aa-helper.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 49e12b9..c22aa66 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -740,6 +740,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi bool readonly = true; bool explicit_deny_rule = true; char *sub = NULL; + char *perms_new = strdup(perms); if (path == NULL) return rc; @@ -764,12 +765,12 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi return rc; } - if (strchr(perms, 'w') != NULL) { + if (strchr(perms_new, 'w') != NULL) { readonly = false; explicit_deny_rule = false; } - if ((sub = strchr(perms, 'R')) != NULL) { + if ((sub = strchr(perms_new, 'R')) != NULL) { /* Don't write the invalid R permission, replace it with 'r' */ sub[0] = 'r'; explicit_deny_rule = false; @@ -787,7 +788,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi if (tmp[strlen(tmp) - 1] == '/') tmp[strlen(tmp) - 1] = '\0'; - virBufferAsprintf(buf, " \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms); + virBufferAsprintf(buf, " \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms_new); if (explicit_deny_rule) { virBufferAddLit(buf, " # don't audit writes to readonly files\n"); virBufferAsprintf(buf, " deny \"%s%s\" w,\n", tmp, recursive ? "/**" : ""); -- 2.9.3

4 5

[libvirt] [PATCH] Delete extra wrap after vol-reisize error
by Yanqiu Zhang 07 Sep '16

07 Sep '16

This patch is to delete the extra wrap "\n" after failed vol-reisize error, both "Failed to change size of volume to" and "Failed to change size of volume by". For error with wrap, there will be an extra wrap between two errors, such as: (1)# virsh vol-resize --pool default --vol vol-test 5M error: Failed to change size of volume 'vol-test' to 5M error: invalid argument: Can't shrink capacity below current capacity unless shrink flag explicitly specified (2)# virsh vol-resize /var/lib/libvirt/images/volds --shrink --delta 10M error: Failed to change size of volume 'volds' by 10M error: invalid argument: can't shrink capacity below existing allocation --- tools/virsh-volume.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index d35fee1..4b3b4d7 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1128,8 +1128,8 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) ret = true; } else { vshError(ctl, - delta ? _("Failed to change size of volume '%s' by %s\n") - : _("Failed to change size of volume '%s' to %s\n"), + delta ? _("Failed to change size of volume '%s' by %s") + : _("Failed to change size of volume '%s' to %s"), virStorageVolGetName(vol), capacityStr); ret = false; } -- 1.8.3.1

2 1

[libvirt] [libvirt-php][PATCH] libvirt_list_active_domains: Don't free domain before strduping its name
by Michal Privoznik 07 Sep '16

07 Sep '16

There's a bug in the implementation of libvirt_list_active_domains(). We try to return an array of names of active domains. So we iterate over array of domains as returned by libvirt and get each domain's name. But we don't obtain a duplicate of that name rather just a pointer into virDomain object. The very next thing we do is we free the virDomain object and then try to copy what's now invalid pointer. Reverse the order of those two actions. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/libvirt-php.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 2045c59..8edcb10 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -9128,18 +9128,15 @@ PHP_FUNCTION(libvirt_list_active_domains) domain=virDomainLookupByID(conn->conn,ids[i]); if (domain!=NULL) { - name=virDomainGetName(domain); - - if (virDomainFree (domain)) - resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, domain, 0 TSRMLS_CC); - - if (name==NULL) - { - efree (ids); + if (!(name = virDomainGetName(domain))) { + efree(ids); RETURN_FALSE; } VIRT_ADD_NEXT_INDEX_STRING(return_value, name); + + if (virDomainFree(domain)) + resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, domain, 0 TSRMLS_CC); } } efree(ids); -- 2.8.4

2 1