February 2013 - Devel - libvirt List Archives

[libvirt] [PATCH] tests: fix leaks in storagevolxml2argvtest
by Ján Tomko 27 Feb '13

27 Feb '13

Reported by John Ferlan. --- tests/storagevolxml2argvtest.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 416bb6f..77ba800 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -89,6 +89,10 @@ cleanup: virCommandFree(cmd); VIR_FREE(actualCmdline); VIR_FREE(expectedCmdline); + VIR_FREE(poolXmlData); + VIR_FREE(volXmlData); + VIR_FREE(inputvolXmlData); + virObjectUnref(conn); return ret; } -- 1.7.12.4

2 2

[libvirt] [PATCH] libvirt.c: Make VIR_MIGRATE_NON_SHARED_{INC, DISK} mutually exclusive
by Michal Privoznik 27 Feb '13

27 Feb '13

These two flags in fact are mutually exclusive. Requesting them both doesn't make any sense regardless of hypervisor driver. Hence, we have to make it within libvirt.c file instead of fixing it in each driver. --- src/libvirt.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/libvirt.c b/src/libvirt.c index 934997a..63416aa 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5161,6 +5161,10 @@ virDomainMigrateDirect(virDomainPtr domain, * XML includes details of the support URI schemes. If omitted * the dconn will be asked for a default URI. * + * If you want to copy non-shared storage within migration you + * can use either VIR_MIGRATE_NON_SHARED_DISK or + * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive. + * * In either case it is typically only necessary to specify a * URI if the destination host has multiple interfaces and a * specific interface is required to transmit migration data. @@ -5221,6 +5225,15 @@ virDomainMigrate(virDomainPtr domain, goto error; } + if (flags & VIR_MIGRATE_NON_SHARED_DISK && + flags & VIR_MIGRATE_NON_SHARED_INC) { + virReportInvalidArg(flags, + _("flags 'shared disk' and 'shared incremental' " + "in %s are mutually exclusive"), + __FUNCTION__); + goto error; + } + if (flags & VIR_MIGRATE_OFFLINE) { if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_OFFLINE)) { @@ -5375,6 +5388,10 @@ error: * XML includes details of the support URI schemes. If omitted * the dconn will be asked for a default URI. * + * If you want to copy non-shared storage within migration you + * can use either VIR_MIGRATE_NON_SHARED_DISK or + * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive. + * * In either case it is typically only necessary to specify a * URI if the destination host has multiple interfaces and a * specific interface is required to transmit migration data. @@ -5448,6 +5465,15 @@ virDomainMigrate2(virDomainPtr domain, goto error; } + if (flags & VIR_MIGRATE_NON_SHARED_DISK && + flags & VIR_MIGRATE_NON_SHARED_INC) { + virReportInvalidArg(flags, + _("flags 'shared disk' and 'shared incremental' " + "in %s are mutually exclusive"), + __FUNCTION__); + goto error; + } + if (flags & VIR_MIGRATE_OFFLINE) { if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_OFFLINE)) { @@ -5601,6 +5627,10 @@ error: * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * + * If you want to copy non-shared storage within migration you + * can use either VIR_MIGRATE_NON_SHARED_DISK or + * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive. + * * If a hypervisor supports renaming domains during migration, * the dname parameter specifies the new name for the domain. * Setting dname to NULL keeps the domain name the same. If domain @@ -5648,6 +5678,15 @@ virDomainMigrateToURI(virDomainPtr domain, virCheckNonNullArgGoto(duri, error); + if (flags & VIR_MIGRATE_NON_SHARED_DISK && + flags & VIR_MIGRATE_NON_SHARED_INC) { + virReportInvalidArg(flags, + _("flags 'shared disk' and 'shared incremental' " + "in %s are mutually exclusive"), + __FUNCTION__); + goto error; + } + if (flags & VIR_MIGRATE_OFFLINE && !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_OFFLINE)) { @@ -5741,6 +5780,10 @@ error: * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * + * If you want to copy non-shared storage within migration you + * can use either VIR_MIGRATE_NON_SHARED_DISK or + * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive. + * * If a hypervisor supports changing the configuration of the guest * during migration, the @dxml parameter specifies the new config * for the guest. The configuration must include an identical set @@ -5799,6 +5842,15 @@ virDomainMigrateToURI2(virDomainPtr domain, goto error; } + if (flags & VIR_MIGRATE_NON_SHARED_DISK && + flags & VIR_MIGRATE_NON_SHARED_INC) { + virReportInvalidArg(flags, + _("flags 'shared disk' and 'shared incremental' " + "in %s are mutually exclusive"), + __FUNCTION__); + goto error; + } + if (flags & VIR_MIGRATE_PEER2PEER) { if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_P2P)) { -- 1.8.1.4

2 2

[libvirt] [PATCH] add dtb support
by Olivia Yin 27 Feb '13

27 Feb '13

Signed-off-by: Olivia Yin <hong-hua.yin(a)freescale.com> --- src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 6 ++++++ 3 files changed, 11 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0c75838..07ad6b9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1771,6 +1771,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->os.kernel); VIR_FREE(def->os.initrd); VIR_FREE(def->os.cmdline); + VIR_FREE(def->os.dtb); VIR_FREE(def->os.root); VIR_FREE(def->os.loader); VIR_FREE(def->os.bootloader); @@ -9993,6 +9994,7 @@ virDomainDefParseXML(virCapsPtr caps, def->os.kernel = virXPathString("string(./os/kernel[1])", ctxt); def->os.initrd = virXPathString("string(./os/initrd[1])", ctxt); def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt); + def->os.dtb = virXPathString("string(./os/dtb[1])", ctxt); def->os.root = virXPathString("string(./os/root[1])", ctxt); def->os.loader = virXPathString("string(./os/loader[1])", ctxt); } @@ -14506,6 +14508,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->os.initrd); virBufferEscapeString(buf, " <cmdline>%s</cmdline>\n", def->os.cmdline); + virBufferEscapeString(buf, " <dtb>%s</dtb>\n", + def->os.dtb); virBufferEscapeString(buf, " <root>%s</root>\n", def->os.root); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4ffa4aa..892640f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1545,6 +1545,7 @@ struct _virDomainOSDef { char *kernel; char *initrd; char *cmdline; + char *dtb; char *root; char *loader; char *bootloader; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dee493f..0c68778 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5628,6 +5628,8 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL); if (def->os.cmdline) virCommandAddArgList(cmd, "-append", def->os.cmdline, NULL); + if (def->os.dtb) + virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL); } else { virCommandAddArgList(cmd, "-bootloader", def->os.bootloader, NULL); } @@ -8794,6 +8796,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, WANT_VALUE(); if (!(def->os.cmdline = strdup(val))) goto no_memory; + } else if (STREQ(arg, "-dtb")) { + WANT_VALUE(); + if (!(def->os.dtb = strdup(val))) + goto no_memory; } else if (STREQ(arg, "-boot")) { const char *token = NULL; WANT_VALUE(); -- 1.6.4

3 2

[libvirt] [PATCH 0/4] doc: storage.html updates
by Philipp Hahn 27 Feb '13

27 Feb '13

Hello, some documentation updates for new storage pools and volumes. Philipp Hahn (4): doc: document new storage volume/pool types doc: add storage format entries doc: fix writing of QEMU doc: white space changes docs/storage.html.in | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) -- 1.7.10.4

2 8

[libvirt] [PATCH v2 0/2] QEMU: add -dtb option support
by Olivia Yin 27 Feb '13

27 Feb '13

Sometime qemu need load specific device tree binary image. These patches provides -dtb option support and update docs/tests. Olivia Yin (2): QEMU: add -dtb option support update documents and test cases for -dtb support src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 6 ++++++ docs/formatdomain.html.in | 5 ++++ docs/schemas/domaincommon.rng | 5 ++++ tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml | 28 ++++++++++++++++++++++

1 2

[libvirt] Problem about CPU index
by Li Zhang 26 Feb '13

26 Feb '13

Hi all, In this function qemuMonitorJSONExtractCPUInfo(), It assumes that cpu index is always contiguous. if (cpu != i) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected cpu index %d expecting %d"), i, cpu); goto cleanup; } For x86, in what kind of situation does this error happen? This assumption causes the problem for ppc64. For ppc64, it's quite different from x86. SMT is needed to be disabled because of hardware's limitation. So, only one thread per core is online. These threads are not contiguous. For example, smt=4, 64 CPUs and 16 Cores CPU information is as the following: Architecture: ppc64 Byte Order: Big Endian CPU(s): 64 On-line CPU(s) list: 0,4,8,12,16,20,24,28,32,36,40,44,48,52,56,60 Off-line CPU(s) list: 1-3,5-7,9-11,13-15,17-19,21-23,25-27,29-31,33-35,37-39,41-43,45-47,49-51,53-55,57-59,61-63 Thread(s) per core: 1 Core(s) per socket: 1 Socket(s): 16 NUMA node(s): 2 All CPUs we get are only 0,4, ..., 60. If possible, can this assumption be removed? Thanks. -- Li Zhang IBM China Linux Technology Centre

3 2

[libvirt] How to trigger autoheader after configure.ac changes?
by TJ 26 Feb '13

26 Feb '13

I'm currently adding support for running a DHCP relay daemon instead of using dnsmasq for DHCP. Everything is looking good but I've stumbled on one issue. I've added a new variable to 'configure.a' (DHCPRELAY) but found that 'autogen.sh' doesn't call 'autoheader' to remake 'config.h.in' so it contains the definition of DHCPRELAY for the compiler. Reading 'autogen.sh' and 'bootstrap' I can see convoluted functionality but can't figure out what the recommended best practice would be in terms of generating the new 'config.h.in'. If I call 'autoheader' manually the definition is added correctly. Should I just do that and then add the changed 'config.h.in' to the commit that introduces the new definition?

1 1

[libvirt] [PATCH] schema: Restrict mode to ocal
by Philipp Hahn 26 Feb '13

26 Feb '13

virStrToLong(..., 8, ...) already requires the mode to be octal. Change the relax-ng schema to check for octal as well. Signed-off-by: Philipp Hahn <hahn(a)univention.de> --- docs/schemas/basictypes.rng | 6 ++++++ docs/schemas/storagepool.rng | 2 +- docs/schemas/storagevol.rng | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index ec1d940..e6cf907 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -20,6 +20,12 @@ </data> </define> + <define name='octalMode'> + <data type="unsignedInt"> + <param name='pattern'>[0-7]+</param> + </data> + </define> + <define name="uint8range"> <choice> <data type="string"> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 165e276..2b1f08d 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -175,7 +175,7 @@ <optional> <element name='permissions'> <element name='mode'> - <ref name='unsignedInt'/> + <ref name='octalMode'/> </element> <element name='owner'> <choice> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 10b7847..d4a29c7 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -46,7 +46,7 @@ <optional> <element name='permissions'> <element name='mode'> - <ref name='unsignedInt'/> + <ref name='octalMode'/> </element> <element name='owner'> <ref name='unsignedInt'/> -- 1.7.10.4

2 1

[libvirt] [PATCH] tests: skip virstoragetest on RHEL 5
by Eric Blake 26 Feb '13

26 Feb '13

virstoragetest was failing on RHEL 5, but with no good error message: TEST: virstoragetest 0 FAIL It turns out that qemu-img was so old, that it lacked support for -o backing_file. It didn't help that the test was also using qemu-img from PATH, even after first probing for kvm-img. * tests/virstoragetest.c (testPrepImages): Consistently use discovered binary. Skip instead of fail if qemu-img fails during setup. --- Pushing under the build-breaker rule; this gets RHEL 5 from 5 test failures down to 4. tests/virstoragetest.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 6ca7b9a..d495e6a 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -89,10 +89,8 @@ testPrepImages(void) qemuimg = virFindFileInPath("kvm-img"); if (!qemuimg) qemuimg = virFindFileInPath("qemu-img"); - if (!qemuimg) { - fputs("qemu-img missing or too old; skipping this test\n", stderr); - return EXIT_AM_SKIP; - } + if (!qemuimg) + goto skip; if (virAsprintf(&absraw, "%s/raw", datadir) < 0 || virAsprintf(&absqcow2, "%s/qcow2", datadir) < 0 || @@ -116,8 +114,10 @@ testPrepImages(void) /* I'm lazy enough to use a shell one-liner instead of open/write/close */ virCommandFree(cmd); cmd = virCommandNewArgList("sh", "-c", "printf %1024d 0 > raw", NULL); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, NULL) < 0) { + fprintf(stderr, "unable to create raw file\n"); goto cleanup; + } if (!(canonraw = canonicalize_file_name(absraw))) { virReportOOMError(); goto cleanup; @@ -126,20 +126,17 @@ testPrepImages(void) /* Create a qcow2 wrapping relative raw; later on, we modify its * metadata to test other configurations */ virCommandFree(cmd); - cmd = virCommandNewArgList("qemu-img", "create", "-f", "qcow2", + cmd = virCommandNewArgList(qemuimg, "create", "-f", "qcow2", "-obacking_file=raw,backing_fmt=raw", "qcow2", NULL); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + goto skip; /* Make sure our later uses of 'qemu-img rebase' will work */ virCommandFree(cmd); - cmd = virCommandNewArgList("qemu-img", "rebase", "-u", "-f", "qcow2", + cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2", "-F", "raw", "-b", "raw", "qcow2", NULL); - if (virCommandRun(cmd, NULL) < 0) { - fputs("qemu-img is too old; skipping this test\n", stderr); - ret = EXIT_AM_SKIP; - goto cleanup; - } + if (virCommandRun(cmd, NULL) < 0) + goto skip; if (!(canonqcow2 = canonicalize_file_name(absqcow2))) { virReportOOMError(); goto cleanup; @@ -148,21 +145,21 @@ testPrepImages(void) /* Create a second qcow2 wrapping the first, to be sure that we * can correctly avoid insecure probing. */ virCommandFree(cmd); - cmd = virCommandNewArgList("qemu-img", "create", "-f", "qcow2", NULL); + cmd = virCommandNewArgList(qemuimg, "create", "-f", "qcow2", NULL); virCommandAddArgFormat(cmd, "-obacking_file=%s,backing_fmt=qcow2", absqcow2); virCommandAddArg(cmd, "wrap"); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + goto skip; /* Create a qed file. */ virCommandFree(cmd); - cmd = virCommandNewArgList("qemu-img", "create", "-f", "qed", NULL); + cmd = virCommandNewArgList(qemuimg, "create", "-f", "qed", NULL); virCommandAddArgFormat(cmd, "-obacking_file=%s,backing_fmt=raw", absraw); virCommandAddArg(cmd, "qed"); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + goto skip; /* Create some symlinks in a sub-directory. */ if (symlink("../qcow2", datadir "/sub/link1") < 0 || @@ -177,6 +174,11 @@ cleanup: if (ret) testCleanupImages(); return ret; + +skip: + fputs("qemu-img is too old; skipping this test\n", stderr); + ret = EXIT_AM_SKIP; + goto cleanup; } typedef struct _testFileData testFileData; -- 1.8.1.2

1 0

[libvirt] [PATCH] interface: udev backend coverity NULL deref
by Doug Goldstein 26 Feb '13

26 Feb '13

This fixes a potential NULL deref identified by John Ferlan <jferlan(a)redhat.com> if scandir() didn't return an expected value. --- src/interface/interface_backend_udev.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index dca85b3..1034429 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -779,6 +779,13 @@ udevIfaceGetIfaceDefBond(struct udev *udev, * so we use the part after the _ */ tmp_str = strchr(slave_list[i]->d_name, '_'); + if (!tmp_str || strlen(tmp_str) < 2) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid enslaved interface name '%s' seen for " + "bond '%s'", slave_list[i]->d_name, name)); + goto cleanup; + } + /* go past the _ */ tmp_str++; ifacedef->data.bond.itf[i] = -- 1.7.12.4

3 2