March 2012 - Devel - libvirt List Archives

[libvirt] [PATCH] util: add stub pciConfigAddressToSysfsFile for non-linux platforms
by Laine Stump 08 Mar '12

08 Mar '12

Absence of this stub function caused a build failure on mingw32. --- Pushed under build breaker rule (I'm doing way too many of these - I really need to start running autobuild before any push, instead of just make check && make syntax-check.) src/util/pci.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index a6212f2..e7bfb92 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -2209,6 +2209,15 @@ pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED, } int +pciConfigAddressToSysfsFile(struct pci_config_address *dev ATTRIBUTE_UNUSED, + char **pci_sysfs_device_link ATTRIBUTE_UNUSED) +{ + pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciConfigAddressToSysfsFile is not " + "supported on non-linux platforms")); + return -1; +} + +int pciDeviceNetName(char *device_link_sysfs_path ATTRIBUTE_UNUSED, char **netname ATTRIBUTE_UNUSED) { -- 1.7.7.6

1 0

[libvirt] Build error on OSX in src/util/virnetlink.c
by Duncan Rance 08 Mar '12

08 Mar '12

Hi, I'm building on OSX with no libnl. I had to do this to get src/util/virnetlink.c to compile: diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 1575bad..59f3e39 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -545,9 +545,9 @@ int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED, */ int virNetlinkEventServiceStop(void) { +# if defined(__linux__) && !defined(HAVE_LIBNL) netlinkError(VIR_ERR_INTERNAL_ERROR, "%s", -# if defined(__linux__) && !defined(HAVE_LIBNL) _("virNetlinkEventServiceStop is not supported since libnl was not available")); # endif return 0; Cheers

3 2

[libvirt] [libvirt-glib] Keep domain devices list sorted
by Zeeshan Ali (Khattak) 08 Mar '12

08 Mar '12

From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org> While we don't guarantee the order of devices in the list returned from gvir_domain_get_devices(), its not a bad idea to keep them sorted the same way we get them from configuration (XML). --- libvirt-gobject/libvirt-gobject-domain.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 7b2c7ad..0bafa7e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -909,5 +909,5 @@ GList *gvir_domain_get_devices(GVirDomain *domain, } g_list_free (config_devices); - return ret; + return g_list_reverse (ret); } -- 1.7.7.6

4 4

[libvirt] [PATCH] rpc: Fix VPATH building issue
by Guannan Ren 08 Mar '12

08 Mar '12

The XDR routine .c file generated by rpcgen includes the corresponding Header file. however, the path in include directive of the header file is picked up based on the path of .x file. In the situation of VPATH Builds it will include full path of the header file rather than relative path. Hence, the error happens when compiling time For example: The libvirt source code resides in /home/testuser, I make dist in /tmp/buildvpath, the XDR routine .c file will include full path of the header file like: #include "/home/testuser/src/rpc/virnetprotocol.h" #include "internal.h" #include <arpa/inet.h> If we distribute the tarball to another machine to compile, it will report error as follows: rpc/virnetprotocol.c:7:59: fatal error: /home/testuser/src/rpc/virnetprotocol.h: No such file or directory --- src/Makefile.am | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index e57eca2..6c9f598 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -662,12 +662,18 @@ $(srcdir)/remote/remote_driver.c: $(REMOTE_DRIVER_GENERATED) endif WITH_REMOTE %protocol.c: %protocol.x %protocol.h $(srcdir)/rpc/genprotocol.pl - $(AM_V_GEN)perl -w $(srcdir)/rpc/genprotocol.pl $(RPCGEN) -c \ - $< $@ + $(AM_V_GEN)protocolx='$<'; \ + protocolc='$@'; \ + cd $(srcdir); \ + perl -w $(srcdir)/rpc/genprotocol.pl $(RPCGEN) -c \ + $${protocolx/'$(srcdir)/'/''} $${protocolc/'$(srcdir)/'/''} %protocol.h: %protocol.x $(srcdir)/rpc/genprotocol.pl - $(AM_V_GEN)perl -w $(srcdir)/rpc/genprotocol.pl $(RPCGEN) -h \ - $< $@ + $(AM_V_GEN)protocolx='$<'; \ + protocolh='$@'; \ + cd $(srcdir); \ + perl -w $(srcdir)/rpc/genprotocol.pl $(RPCGEN) -h \ + $${protocolx/'$(srcdir)/'/''} $${protocolh/'$(srcdir)/'/''} if WITH_XEN if WITH_DRIVER_MODULES -- 1.7.7.5

2 3

[libvirt] [PATCH] xml: Clean up schemas to use shared data types instead of local
by Peter Krempa 08 Mar '12

08 Mar '12

The schema files contained duplicate data types that can be shared from the basictypes.rng file. --- docs/schemas/capability.rng | 43 +++++---------------------------- docs/schemas/interface.rng | 16 ++++-------- docs/schemas/nodedev.rng | 54 +++++++++++++++--------------------------- docs/schemas/nwfilter.rng | 28 +--------------------- 4 files changed, 32 insertions(+), 109 deletions(-) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 3af95e9..06ff685 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -1,6 +1,7 @@  <grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <include href='basictypes.rng'/> <start> <ref name='capabilities'/> </start> @@ -157,7 +158,7 @@ <element name='topology'> <element name='cells'> <attribute name='num'> - <ref name='uint'/> + <ref name='unsignedInt'/> </attribute> <oneOrMore> <ref name='cell'/> @@ -169,13 +170,13 @@ <define name='cell'> <element name='cell'> <attribute name='id'> - <ref name='uint'/> + <ref name='unsignedInt'/> </attribute> <optional> <element name='cpus'> <attribute name='num'> - <ref name='uint'/> + <ref name='unsignedInt'/> </attribute> <oneOrMore> <ref name='cpu'/> @@ -188,7 +189,7 @@ <define name='cpu'> <element name='cpu'> <attribute name='id'> - <ref name='uint'/> + <ref name='unsignedInt'/> </attribute> </element> </define> @@ -238,13 +239,13 @@ <define name='emulator'> <element name='emulator'> - <ref name='path'/> + <ref name='absFilePath'/> </element> </define> <define name='loader'> <element name='loader'> - <ref name='path'/> + <ref name='absFilePath'/> </element> </define> @@ -367,39 +368,9 @@ </choice> </define> - - <define name='positiveInteger'> - <data type='positiveInteger'> - <param name="pattern">[0-9]+</param> - </data> - </define> - - <define name='uint'> - <data type='unsignedInt'> - <param name="pattern">[0-9]+</param> - </data> - </define> - - <define name='path'> - <data type='string'> - <param name="pattern">/[a-zA-Z0-9_\+\-/%]+</param> - </data> - </define> - <define name='featureName'> <data type='string'> <param name='pattern'>[a-zA-Z0-9\-_]+</param> </data> </define> - - <define name="UUID"> - <choice> - <data type="string"> - <param name="pattern">[a-fA-F0-9]{32}</param> - </data> - <data type="string"> - <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> - </data> - </choice> - </define> </grammar> diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index 53fa18a..3984b63 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -183,12 +183,12 @@ <choice> <element name="miimon">  - <attribute name="freq"><ref name="uint"/></attribute> + <attribute name="freq"><ref name="unsignedInt"/></attribute> <optional> - <attribute name="downdelay"><ref name="uint"/></attribute> + <attribute name="downdelay"><ref name="unsignedInt"/></attribute> </optional> <optional> - <attribute name="updelay"><ref name="uint"/></attribute> + <attribute name="updelay"><ref name="unsignedInt"/></attribute> </optional> <optional>  @@ -203,7 +203,7 @@ </optional> </element> <element name="arpmon"> - <attribute name="interval"><ref name="uint"/></attribute> + <attribute name="interval"><ref name="unsignedInt"/></attribute> <attribute name="target"><ref name="ipv4Addr"/></attribute> <optional> <attribute name="validate"> @@ -252,7 +252,7 @@ <define name="mtu"> <optional> <element name="mtu"> - <attribute name="size"><ref name="uint"/></attribute> + <attribute name="size"><ref name="unsignedInt"/></attribute> </element> </optional> </define> @@ -407,12 +407,6 @@  - <define name='uint'> - <data type='unsignedInt'> - <param name="pattern">[0-9]+</param> - </data> - </define> - <define name="timeval"> <data type="double"> <param name="minInclusive">0</param> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 1b9a2d1..a73c2e5 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -1,6 +1,7 @@  <grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <include href='basictypes.rng'/> <start> <ref name='device'/> </start> @@ -56,7 +57,7 @@ </optional> <element name='uuid'> - <ref name='uuid'/> + <ref name='UUID'/> </element> </element> @@ -80,16 +81,16 @@ </attribute> <element name='domain'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='bus'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='slot'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='function'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='product'> @@ -122,10 +123,10 @@ </attribute> <element name='bus'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='device'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='product'> @@ -157,16 +158,16 @@ </attribute> <element name='number'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='class'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='subclass'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='protocol'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <optional> @@ -252,7 +253,7 @@ </attribute> <element name='host'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <optional> @@ -273,16 +274,16 @@ </attribute> <element name='host'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='bus'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='target'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='lun'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <element name='type'> @@ -352,7 +353,7 @@ </element> <element name='media_size'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> <optional> <element name='media_label'> @@ -364,27 +365,10 @@ <define name='capstoragefixed'> <element name='size'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> </define> - <define name='uuid'> - <choice> - <data type='string'> - <param name="pattern">[a-fA-F0-9]{32}</param> - </data> - <data type='string'> - <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> - </data> - </choice> - </define> - - <define name='uint'> - <data type='string'> - <param name="pattern">[0-9]+</param> - </data> - </define> - <define name='hexuint'> <data type='string'> <param name="pattern">(0x)?[0-9a-f]+</param> diff --git a/docs/schemas/nwfilter.rng b/docs/schemas/nwfilter.rng index bce3d3b..13fd9b9 100644 --- a/docs/schemas/nwfilter.rng +++ b/docs/schemas/nwfilter.rng @@ -1,5 +1,6 @@ <?xml version="1.0" encoding="UTF-8"?> <grammar ns="" xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <include href='basictypes.rng'/> <start> <ref name="filter"/> </start> @@ -799,18 +800,6 @@  - <define name="UUID"> - <choice> - <data type="string"> - <param name="pattern">[a-fA-F0-9]{32}</param> - </data> - - <data type="string"> - <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> - </data> - </choice> - </define> - <define name="variable-name-type"> <data type="string"> <param name="pattern">$[a-zA-Z0-9_]+(\[[ ]*[@]?[0-9]+[ ]*\])?</param> @@ -930,21 +919,6 @@ </choice> </define> - <define name="uint8range"> - <choice> - <ref name="variable-name-type"/> - - <data type="string"> - <param name="pattern">0x[0-9a-fA-F]{1,2}</param> - </data> - - <data type="int"> - <param name="minInclusive">0</param> - <param name="maxInclusive">255</param> - </data> - </choice> - </define> - <define name="uint16range"> <choice> <ref name="variable-name-type"/> -- 1.7.3.4

3 3

[libvirt] [PATCH] util: Don't overflow on errno in virFileAccessibleAs
by Michal Privoznik 08 Mar '12

08 Mar '12

If we need to virFork() to check assess() under different UID+GID we need to translate returned status via WEXITSTATUS(). Otherwise, we may return values greater than 255 which is obviously wrong. --- src/util/util.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 548ed1c..15e6cfa 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -724,8 +724,13 @@ virFileAccessibleAs(const char *path, int mode, return -1; } + if (!WIFEXITED(status)) { + errno = EINTR; + return -1; + } + if (status) { - errno = status; + errno = WEXITSTATUS(status); return -1; } -- 1.7.8.5

2 2

[libvirt] [PATCH] qemu: Fix startupPolicy for snapshot-revert
by Michal Privoznik 08 Mar '12

08 Mar '12

Currently, startupPolicy='requisite' was determining cold boot by migrateForm != NULL. That means, if domain was started up with migrateForm set we didn't require disk source path and allowed it to be dropped. However, on snapshot-revert domain wasn't migrated but according to documentation, requisite should drop disk source as well. --- Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=798938 src/qemu/qemu_driver.c | 16 +++++++++------- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_process.c | 3 ++- src/qemu/qemu_process.h | 1 + 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ddb381a..b17b52c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1358,7 +1358,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; /* XXXX free the 'vm' we created ? */ - if (qemuProcessStart(conn, driver, vm, NULL, + if (qemuProcessStart(conn, driver, vm, NULL, true, (flags & VIR_DOMAIN_START_PAUSED) != 0, (flags & VIR_DOMAIN_START_AUTODESTROY) != 0, -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE) < 0) { @@ -4107,8 +4107,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ - ret = qemuProcessStart(conn, driver, vm, "stdio", true, - false, *fd, path, NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE); + ret = qemuProcessStart(conn, driver, vm, "stdio", false, true, + false, *fd, path, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_RESTORE); if (intermediatefd != -1) { if (ret < 0) { @@ -4709,8 +4710,9 @@ qemuDomainObjStart(virConnectPtr conn, } } - ret = qemuProcessStart(conn, driver, vm, NULL, start_paused, - autodestroy, -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE); + ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused, + autodestroy, -1, NULL, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE); virDomainAuditStart(vm, "booted", ret >= 0); if (ret >= 0) { virDomainEventPtr event = @@ -10788,7 +10790,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainObjAssignDef(vm, config, false); rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL, - true, false, -1, NULL, snap, + false, true, false, -1, NULL, snap, VIR_NETDEV_VPORT_PROFILE_OP_CREATE); virDomainAuditStart(vm, "from-snapshot", rc >= 0); detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; @@ -10878,7 +10880,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (event) qemuDomainEventQueue(driver, event); rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL, - paused, false, -1, NULL, NULL, + false, paused, false, -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE); virDomainAuditStart(vm, "from-snapshot", rc >= 0); if (rc < 0) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 77d40c0..92d046a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1229,7 +1229,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, /* Start the QEMU daemon, with the same command-line arguments plus * -incoming $migrateFrom */ - if (qemuProcessStart(dconn, driver, vm, migrateFrom, true, + if (qemuProcessStart(dconn, driver, vm, migrateFrom, false, true, true, dataFD[0], NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) < 0) { virDomainAuditStart(vm, "migrated", false); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7b99814..502de89 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3072,6 +3072,7 @@ int qemuProcessStart(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, const char *migrateFrom, + bool cold_boot, bool start_paused, bool autodestroy, int stdin_fd, @@ -3227,7 +3228,7 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; VIR_DEBUG("Checking for CDROM and floppy presence"); - if (qemuDomainCheckDiskPresence(driver, vm, migrateFrom != NULL) < 0) + if (qemuDomainCheckDiskPresence(driver, vm, cold_boot) < 0) goto cleanup; VIR_DEBUG("Setting up domain cgroup (if required)"); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 2c1d0b5..761db6f 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -48,6 +48,7 @@ int qemuProcessStart(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, const char *migrateFrom, + bool cold_boot, bool start_paused, bool autodestroy, int stdin_fd, -- 1.7.8.5

2 3

[libvirt] [PATCH] sanlock: Use STREQ_NULLABLE instead of STREQ on strings that may be null
by Peter Krempa 08 Mar '12

08 Mar '12

The function sanlock_inquire can return NULL in the state string if the message consists only of a header. The return value is arbitrary and sent by the server. We should proceed carefully while touching such pointers. --- src/locking/lock_driver_sanlock.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index 13940f1..f2623a0 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -828,7 +828,7 @@ static int virLockManagerSanlockRelease(virLockManagerPtr lock, return -1; } - if (STREQ(*state, "")) + if (STREQ_NULLABLE(*state, "")) VIR_FREE(*state); } @@ -871,7 +871,7 @@ static int virLockManagerSanlockInquire(virLockManagerPtr lock, return -1; } - if (STREQ(*state, "")) + if (STREQ_NULLABLE(*state, "")) VIR_FREE(*state); return 0; -- 1.7.3.4

2 2

[libvirt] [PATCH] sanlock: Fix condition left crippled while debugging
by Peter Krempa 08 Mar '12

08 Mar '12

--- Extra context for review. src/locking/lock_driver_sanlock.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index f2623a0..d344d6a 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -669,73 +669,73 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, const char *state, unsigned int flags, int *fd) { virLockManagerSanlockPrivatePtr priv = lock->privateData; struct sanlk_options *opt; struct sanlk_resource **res_args; int res_count; bool res_free = false; int sock = -1; int rv; int i; virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_RESTRICT | VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1); if (priv->res_count == 0 && priv->hasRWDisks && driver->requireLeaseForDisks) { virLockError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Read/write, exclusive access, disks were present, but no leases specified")); return -1; } if (VIR_ALLOC(opt) < 0) { virReportOOMError(); return -1; } if (!virStrcpy(opt->owner_name, priv->vm_name, SANLK_NAME_LEN)) { virLockError(VIR_ERR_INTERNAL_ERROR, _("Domain name '%s' exceeded %d characters"), priv->vm_name, SANLK_NAME_LEN); goto error; } - if (state && STRNEQ(state, "") && 0) { + if (state && STRNEQ(state, "")) { if ((rv = sanlock_state_to_args((char *)state, &res_count, &res_args)) < 0) { if (rv <= -200) virLockError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse lock state %s: error %d"), state, rv); else virReportSystemError(-rv, _("Unable to parse lock state %s"), state); goto error; } res_free = true; } else { res_args = priv->res_args; res_count = priv->res_count; } VIR_DEBUG("Register sanlock %d", flags); /* We only initialize 'sock' if we are in the real * child process and we need it to be inherited * * If sock==-1, then sanlock auto-open/closes a * temporary sock */ if (priv->vm_pid == getpid() && (sock = sanlock_register()) < 0) { if (sock <= -200) virLockError(VIR_ERR_INTERNAL_ERROR, _("Failed to open socket to sanlock daemon: error %d"), sock); else virReportSystemError(-sock, "%s", _("Failed to open socket to sanlock daemon")); goto error; -- 1.7.3.4

2 2

[libvirt] [PATCH] qemu: Don't parse device twice in attach/detach
by Michal Privoznik 08 Mar '12

08 Mar '12

Some nits are generated during XML parse (e.g. MAC address of an interface); However, with current implementation, if we are plugging a device both to persistent and live config, we parse given XML twice: first time for live, second for config. This is wrong then as the second time we are not guaranteed to generate same values as we did for the first time. To prevent that we need to create a copy of DeviceDefPtr; This is done through format/parse process instead of writing functions for deep copy as it is easier to maintain: adding new field to any virDomain*DefPtr doesn't require change of copying function. --- src/conf/domain_conf.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 40 +++++++++++-------- 4 files changed, 121 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f9654f1..f001319 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14118,3 +14118,97 @@ virDomainNetFind(virDomainDefPtr def, const char *device) return net; } + +#define VIR_DOMAIN_DEVICE_FORMAT(func) \ + if (func < 0) \ + goto cleanup; \ + xmlStr = virBufferContentAndReset(&buf); \ + ret = virDomainDeviceDefParse(caps, def, xmlStr, flags) + +virDomainDeviceDefPtr +virDomainDeviceDefCopy(virCapsPtr caps, + const virDomainDefPtr def, + virDomainDeviceDefPtr src) +{ + virDomainDeviceDefPtr ret = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + int flags = VIR_DOMAIN_XML_INACTIVE; + char *xmlStr = NULL; + + switch(src->type) { + case VIR_DOMAIN_DEVICE_DISK: + VIR_DOMAIN_DEVICE_FORMAT(virDomainDiskDefFormat(&buf, + src->data.disk, + flags)); + break; + case VIR_DOMAIN_DEVICE_LEASE: + VIR_DOMAIN_DEVICE_FORMAT(virDomainLeaseDefFormat(&buf, + src->data.lease)); + break; + case VIR_DOMAIN_DEVICE_FS: + VIR_DOMAIN_DEVICE_FORMAT(virDomainFSDefFormat(&buf, + src->data.fs, + flags)); + break; + case VIR_DOMAIN_DEVICE_NET: + VIR_DOMAIN_DEVICE_FORMAT(virDomainNetDefFormat(&buf, + src->data.net, + flags)); + break; + case VIR_DOMAIN_DEVICE_INPUT: + VIR_DOMAIN_DEVICE_FORMAT(virDomainInputDefFormat(&buf, + src->data.input, + flags)); + break; + case VIR_DOMAIN_DEVICE_SOUND: + VIR_DOMAIN_DEVICE_FORMAT(virDomainSoundDefFormat(&buf, + src->data.sound, + flags)); + break; + case VIR_DOMAIN_DEVICE_VIDEO: + VIR_DOMAIN_DEVICE_FORMAT(virDomainVideoDefFormat(&buf, + src->data.video, + flags)); + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: + VIR_DOMAIN_DEVICE_FORMAT(virDomainHostdevDefFormat(&buf, + src->data.hostdev, + flags)); + break; + case VIR_DOMAIN_DEVICE_WATCHDOG: + VIR_DOMAIN_DEVICE_FORMAT(virDomainWatchdogDefFormat(&buf, + src->data.watchdog, + flags)); + break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + VIR_DOMAIN_DEVICE_FORMAT(virDomainControllerDefFormat(&buf, + src->data.controller, + flags)); + break; + case VIR_DOMAIN_DEVICE_GRAPHICS: + VIR_DOMAIN_DEVICE_FORMAT(virDomainGraphicsDefFormat(&buf, + src->data.graphics, + flags)); + break; + case VIR_DOMAIN_DEVICE_HUB: + VIR_DOMAIN_DEVICE_FORMAT(virDomainHubDefFormat(&buf, + src->data.hub, + flags)); + break; + case VIR_DOMAIN_DEVICE_REDIRDEV: + VIR_DOMAIN_DEVICE_FORMAT(virDomainRedirdevDefFormat(&buf, + src->data.redirdev, + flags)); + break; + default: + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("Copying definition of '%d' type " + "is not implemented yet."), + src->type); + break; + } + +cleanup: + VIR_FREE(xmlStr); + return ret; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 596be4d..7e4a464 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1750,6 +1750,9 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def); void virDomainHubDefFree(virDomainHubDefPtr def); void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def); void virDomainDeviceDefFree(virDomainDeviceDefPtr def); +virDomainDeviceDefPtr virDomainDeviceDefCopy(virCapsPtr caps, + const virDomainDefPtr def, + virDomainDeviceDefPtr src); int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a104e70..53f27e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -286,6 +286,7 @@ virDomainDeviceAddressIsValid; virDomainDeviceAddressPciMultiTypeFromString; virDomainDeviceAddressPciMultiTypeToString; virDomainDeviceAddressTypeToString; +virDomainDeviceDefCopy; virDomainDeviceDefFree; virDomainDeviceDefParse; virDomainDeviceInfoIterate; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c6bdd29..3a52ded 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5561,8 +5561,9 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; + bool free_dev_copy = false; int ret = -1; unsigned int affect; @@ -5608,12 +5609,24 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) + dev = dev_copy = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + + if (flags & VIR_DOMAIN_AFFECT_CONFIG && + flags & VIR_DOMAIN_AFFECT_LIVE) { + /* If we are affecting both CONFIG and LIVE + * create a deep copy of device as adding + * to CONFIG takes one instance. + */ + dev_copy = virDomainDeviceDefCopy(driver->caps, vm->def, dev); + if (!dev_copy) goto endjob; + free_dev_copy = true; + } + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(driver->caps, vm); if (!vmdef) @@ -5639,24 +5652,15 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - /* If dev exists it was created to modify the domain config. Free it. */ - virDomainDeviceDefFree(dev); - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) { - ret = -1; - goto endjob; - } - switch (action) { case QEMU_DEVICE_ATTACH: - ret = qemuDomainAttachDeviceLive(vm, dev, dom); + ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom); break; case QEMU_DEVICE_DETACH: - ret = qemuDomainDetachDeviceLive(vm, dev, dom); + ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom); break; case QEMU_DEVICE_UPDATE: - ret = qemuDomainUpdateDeviceLive(vm, dev, dom, force); + ret = qemuDomainUpdateDeviceLive(vm, dev_copy, dom, force); break; default: qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -5694,6 +5698,8 @@ endjob: cleanup: virDomainDefFree(vmdef); virDomainDeviceDefFree(dev); + if (free_dev_copy) + virDomainDeviceDefFree(dev_copy); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); -- 1.7.3.4

3 6