[libvirt] [PATCH 00/12] qemu_process cleanups and test improvement
The first patch is unrelated but I've found it while working on this series. Pavel Hrdina (12): qemu_process: check for correct return value while starting domain nvram: generate it's path in qemuDomainDefPostParse tests: cleanup qemuxml2argvtest qemu_process: introduce qemuProcessPrepareDomain qemu_process: introduce qemuProcessPrepareHost qemu: update callers of qemuProcessStartValidate to use virDomainObjPtr qemu_process: move checks to qemuProcessStartValidate qemu_process: move qemuDomainSetPrivatePaths to qemuProcessInit qemu_process: introduce qemuProcessCreateCmd qemu_driver: cleanup qemuConnectDomainXMLToNative qemuxml2argvtest: use driver.config and priv for qemuDomainSetPrivatePaths qemuxml2argvtest: cleanup test src/qemu/qemu_domain.c | 10 + src/qemu/qemu_driver.c | 61 +- src/qemu/qemu_migration.c | 9 +- src/qemu/qemu_process.c | 706 +++++++++++---------- src/qemu/qemu_process.h | 26 +- .../qemuxml2argv-channel-virtio-unix.xml | 2 +- tests/qemuxml2argvtest.c | 126 ++-- tests/testutilsqemu.c | 7 + 8 files changed, 494 insertions(+), 453 deletions(-) -- 2.7.2
Function qemuProcessLaunch returns '-2' in case there was an error and we need to cleanup labels. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 14e4629..3c496cb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5405,7 +5405,7 @@ qemuProcessStart(virConnectPtr conn, if ((rv = qemuProcessLaunch(conn, driver, vm, asyncJob, incoming, snapshot, vmop, flags)) < 0) { - if (rv == -1) + if (rv == -2) relabel = true; goto stop; } -- 2.7.2
On Tue, Mar 15, 2016 at 14:15:57 +0100, Pavel Hrdina wrote:
Function qemuProcessLaunch returns '-2' in case there was an error and we need to cleanup labels.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 14e4629..3c496cb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5405,7 +5405,7 @@ qemuProcessStart(virConnectPtr conn,
if ((rv = qemuProcessLaunch(conn, driver, vm, asyncJob, incoming, snapshot, vmop, flags)) < 0) { - if (rv == -1) + if (rv == -2) relabel = true; goto stop; }
Oops. ACK. Jirka
The postParse callback is the correct place to generate default values that should be present in offline XML. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++ src/qemu/qemu_process.c | 153 ++++++++++++++++++------------------------------ 2 files changed, 68 insertions(+), 95 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 594063e..632cf47 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1397,6 +1397,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, void *opaque) { virQEMUDriverPtr driver = opaque; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virQEMUCapsPtr qemuCaps = NULL; int ret = -1; @@ -1412,6 +1413,15 @@ qemuDomainDefPostParse(virDomainDefPtr def, return ret; } + if (def->os.loader && + def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH && + def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON && + !def->os.loader->nvram) { + if (virAsprintf(&def->os.loader->nvram, "%s/%s_VARS.fd", + cfg->nvramDir, def->name) < 0) + goto cleanup; + } + /* check for emulator and create a default one if needed */ if (!def->emulator && !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3c496cb..958fae3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3878,7 +3878,6 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, static int qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, - virCapsPtr caps, virDomainObjPtr vm, bool migrated) { @@ -3886,111 +3885,81 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, int srcFD = -1; int dstFD = -1; virDomainLoaderDefPtr loader = vm->def->os.loader; - bool generated = false; bool created = false; + const char *master_nvram_path; + ssize_t r; - /* Unless domain has RO loader of pflash type, we have - * nothing to do here. If the loader is RW then it's not - * using split code and vars feature, so no nvram file needs - * to be created. */ - if (!loader || loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH || - loader->readonly != VIR_TRISTATE_SWITCH_ON) + if (!loader->nvram || !migrated) return 0; - /* If the nvram path is configured already, there's nothing - * we need to do. Unless we are starting the destination side - * of migration in which case nvram is configured in the - * domain XML but the file doesn't exist yet. Moreover, after - * the migration is completed, qemu will invoke a - * synchronization write into the nvram file so we don't have - * to take care about transmitting the real data on the other - * side. */ - if (loader->nvram && !migrated) + if (virFileExists(loader->nvram)) return 0; - /* Autogenerate nvram path if needed.*/ - if (!loader->nvram) { - if (virAsprintf(&loader->nvram, - "%s/%s_VARS.fd", - cfg->nvramDir, vm->def->name) < 0) - goto cleanup; - - generated = true; - - if (vm->persistent && - virDomainSaveConfig(cfg->configDir, caps, vm->def) < 0) - goto cleanup; - } - - if (!virFileExists(loader->nvram)) { - const char *master_nvram_path = loader->templt; - ssize_t r; - - if (!loader->templt) { - size_t i; - for (i = 0; i < cfg->nloader; i++) { - if (STREQ(cfg->loader[i], loader->path)) { - master_nvram_path = cfg->nvram[i]; - break; - } + master_nvram_path = loader->templt; + if (!loader->templt) { + size_t i; + for (i = 0; i < cfg->nloader; i++) { + if (STREQ(cfg->loader[i], loader->path)) { + master_nvram_path = cfg->nvram[i]; + break; } } + } - if (!master_nvram_path) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("unable to find any master var store for " - "loader: %s"), loader->path); - goto cleanup; - } - - if ((srcFD = virFileOpenAs(master_nvram_path, O_RDONLY, - 0, -1, -1, 0)) < 0) { - virReportSystemError(-srcFD, - _("Failed to open file '%s'"), - master_nvram_path); - goto cleanup; - } - if ((dstFD = virFileOpenAs(loader->nvram, - O_WRONLY | O_CREAT | O_EXCL, - S_IRUSR | S_IWUSR, - cfg->user, cfg->group, 0)) < 0) { - virReportSystemError(-dstFD, - _("Failed to create file '%s'"), - loader->nvram); - goto cleanup; - } - created = true; - - do { - char buf[1024]; + if (!master_nvram_path) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("unable to find any master var store for " + "loader: %s"), loader->path); + goto cleanup; + } - if ((r = saferead(srcFD, buf, sizeof(buf))) < 0) { - virReportSystemError(errno, - _("Unable to read from file '%s'"), - master_nvram_path); - goto cleanup; - } + if ((srcFD = virFileOpenAs(master_nvram_path, O_RDONLY, + 0, -1, -1, 0)) < 0) { + virReportSystemError(-srcFD, + _("Failed to open file '%s'"), + master_nvram_path); + goto cleanup; + } + if ((dstFD = virFileOpenAs(loader->nvram, + O_WRONLY | O_CREAT | O_EXCL, + S_IRUSR | S_IWUSR, + cfg->user, cfg->group, 0)) < 0) { + virReportSystemError(-dstFD, + _("Failed to create file '%s'"), + loader->nvram); + goto cleanup; + } + created = true; - if (safewrite(dstFD, buf, r) < 0) { - virReportSystemError(errno, - _("Unable to write to file '%s'"), - loader->nvram); - goto cleanup; - } - } while (r); + do { + char buf[1024]; - if (VIR_CLOSE(srcFD) < 0) { + if ((r = saferead(srcFD, buf, sizeof(buf))) < 0) { virReportSystemError(errno, - _("Unable to close file '%s'"), + _("Unable to read from file '%s'"), master_nvram_path); goto cleanup; } - if (VIR_CLOSE(dstFD) < 0) { + + if (safewrite(dstFD, buf, r) < 0) { virReportSystemError(errno, - _("Unable to close file '%s'"), + _("Unable to write to file '%s'"), loader->nvram); goto cleanup; } + } while (r); + + if (VIR_CLOSE(srcFD) < 0) { + virReportSystemError(errno, + _("Unable to close file '%s'"), + master_nvram_path); + goto cleanup; + } + if (VIR_CLOSE(dstFD) < 0) { + virReportSystemError(errno, + _("Unable to close file '%s'"), + loader->nvram); + goto cleanup; } ret = 0; @@ -4000,8 +3969,6 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, if (ret < 0) { if (created) unlink(loader->nvram); - if (generated) - VIR_FREE(loader->nvram); } VIR_FORCE_CLOSE(srcFD); @@ -4421,13 +4388,6 @@ qemuProcessInit(virQEMUDriverPtr driver, if (qemuProcessStartValidate(vm->def, priv->qemuCaps, migration, snap) < 0) goto cleanup; - /* Some things, paths, ... are generated here and we want them to persist. - * Fill them in prior to setting the domain def as transient. */ - VIR_DEBUG("Generating paths"); - - if (qemuPrepareNVRAM(cfg, caps, vm, migration) < 0) - goto stop; - /* Do this upfront, so any part of the startup process can add * runtime state to vm->def that won't be persisted. This let's us * report implicit runtime defaults in the XML, like vnc listen/socket @@ -4436,6 +4396,9 @@ qemuProcessInit(virQEMUDriverPtr driver, if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, true) < 0) goto stop; + if (qemuPrepareNVRAM(cfg, vm, migration) < 0) + goto stop; + vm->def->id = qemuDriverAllocateID(driver); qemuDomainSetFakeReboot(driver, vm, false); virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); -- 2.7.2
On Tue, Mar 15, 2016 at 14:15:58 +0100, Pavel Hrdina wrote:
The postParse callback is the correct place to generate default values that should be present in offline XML.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++ src/qemu/qemu_process.c | 153 ++++++++++++++++++------------------------------ 2 files changed, 68 insertions(+), 95 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 594063e..632cf47 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1397,6 +1397,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, void *opaque) { virQEMUDriverPtr driver = opaque; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virQEMUCapsPtr qemuCaps = NULL; int ret = -1;
@@ -1412,6 +1413,15 @@ qemuDomainDefPostParse(virDomainDefPtr def, return ret; }
+ if (def->os.loader && + def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH && + def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON && + !def->os.loader->nvram) { + if (virAsprintf(&def->os.loader->nvram, "%s/%s_VARS.fd", + cfg->nvramDir, def->name) < 0) + goto cleanup; + } + /* check for emulator and create a default one if needed */ if (!def->emulator && !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3c496cb..958fae3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3878,7 +3878,6 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
static int qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, - virCapsPtr caps, virDomainObjPtr vm, bool migrated) { @@ -3886,111 +3885,81 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, int srcFD = -1; int dstFD = -1; virDomainLoaderDefPtr loader = vm->def->os.loader; - bool generated = false; bool created = false; + const char *master_nvram_path; + ssize_t r;
- /* Unless domain has RO loader of pflash type, we have - * nothing to do here. If the loader is RW then it's not - * using split code and vars feature, so no nvram file needs - * to be created. */ - if (!loader || loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH || - loader->readonly != VIR_TRISTATE_SWITCH_ON) + if (!loader->nvram || !migrated) return 0;
- /* If the nvram path is configured already, there's nothing - * we need to do. Unless we are starting the destination side - * of migration in which case nvram is configured in the - * domain XML but the file doesn't exist yet. Moreover, after - * the migration is completed, qemu will invoke a - * synchronization write into the nvram file so we don't have - * to take care about transmitting the real data on the other - * side. */ - if (loader->nvram && !migrated) + if (virFileExists(loader->nvram)) return 0;
So previously if loader->nvram != NULL, the file was already created (except for migration). Now you fixed it and generated nvram path has no relation to the existence of the file. It doesn't make any sense to check whether we are migrating or not anymore. I think the check should be if (!loader->nvram || virFileExists(loader->nvram)) return 0; The check you have would only create the nvram file if loader->nvram != NULL and migrated = true, i.e., it won't be created if you start a new domain without migrating it. ACK to the patch if you agree with ^, otherwise an explanation of why your current code is correct will be needed :-) Jirka
On Tue, Mar 22, 2016 at 10:06:19AM +0100, Jiri Denemark wrote:
On Tue, Mar 15, 2016 at 14:15:58 +0100, Pavel Hrdina wrote:
The postParse callback is the correct place to generate default values that should be present in offline XML.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++ src/qemu/qemu_process.c | 153 ++++++++++++++++++------------------------------ 2 files changed, 68 insertions(+), 95 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 594063e..632cf47 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1397,6 +1397,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, void *opaque) { virQEMUDriverPtr driver = opaque; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virQEMUCapsPtr qemuCaps = NULL; int ret = -1;
@@ -1412,6 +1413,15 @@ qemuDomainDefPostParse(virDomainDefPtr def, return ret; }
+ if (def->os.loader && + def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH && + def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON && + !def->os.loader->nvram) { + if (virAsprintf(&def->os.loader->nvram, "%s/%s_VARS.fd", + cfg->nvramDir, def->name) < 0) + goto cleanup; + } + /* check for emulator and create a default one if needed */ if (!def->emulator && !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3c496cb..958fae3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3878,7 +3878,6 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
static int qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, - virCapsPtr caps, virDomainObjPtr vm, bool migrated) { @@ -3886,111 +3885,81 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, int srcFD = -1; int dstFD = -1; virDomainLoaderDefPtr loader = vm->def->os.loader; - bool generated = false; bool created = false; + const char *master_nvram_path; + ssize_t r;
- /* Unless domain has RO loader of pflash type, we have - * nothing to do here. If the loader is RW then it's not - * using split code and vars feature, so no nvram file needs - * to be created. */ - if (!loader || loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH || - loader->readonly != VIR_TRISTATE_SWITCH_ON) + if (!loader->nvram || !migrated) return 0;
- /* If the nvram path is configured already, there's nothing - * we need to do. Unless we are starting the destination side - * of migration in which case nvram is configured in the - * domain XML but the file doesn't exist yet. Moreover, after - * the migration is completed, qemu will invoke a - * synchronization write into the nvram file so we don't have - * to take care about transmitting the real data on the other - * side. */ - if (loader->nvram && !migrated) + if (virFileExists(loader->nvram)) return 0;
So previously if loader->nvram != NULL, the file was already created (except for migration). Now you fixed it and generated nvram path has no relation to the existence of the file. It doesn't make any sense to check whether we are migrating or not anymore. I think the check should be
if (!loader->nvram || virFileExists(loader->nvram)) return 0;
The check you have would only create the nvram file if loader->nvram != NULL and migrated = true, i.e., it won't be created if you start a new domain without migrating it.
ACK to the patch if you agree with ^, otherwise an explanation of why your current code is correct will be needed :-)
Jirka
Nice catch, you're right and this modification fixes the startup of new machine. Thanks, Pavel
This removes the testFailed magic and makes the code more readable. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/qemuxml2argvtest.c | 62 ++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 34 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e15da37..150a50d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -265,7 +265,6 @@ static int testCompareXMLToArgvFiles(const char *xml, virCommandPtr cmd = NULL; size_t i; virBitmapPtr nodeset = NULL; - bool testFailed = false; char *domainLibDir = NULL; char *domainChannelTargetDir = NULL; @@ -280,8 +279,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, (VIR_DOMAIN_DEF_PARSE_INACTIVE | parseFlags)))) { - if (!virtTestOOMActive() && - (flags & FLAG_EXPECT_PARSE_ERROR)) + if (flags & FLAG_EXPECT_PARSE_ERROR) goto ok; goto out; } @@ -349,39 +347,22 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } - if (qemuProcessStartValidate(vmdef, extraFlags, !!migrateURI, false) < 0) - testFailed = true; + if (qemuProcessStartValidate(vmdef, extraFlags, !!migrateURI, false) < 0) { + if (flags & FLAG_EXPECT_FAILURE) + goto ok; + goto out; + } - if (!testFailed && - !(cmd = qemuBuildCommandLine(conn, &driver, NULL, vmdef, &monitor_chr, + if (!(cmd = qemuBuildCommandLine(conn, &driver, NULL, vmdef, &monitor_chr, (flags & FLAG_JSON), extraFlags, migrateURI, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, &testCallbacks, false, (flags & FLAG_FIPS), nodeset, NULL, NULL, - domainLibDir, domainChannelTargetDir))) - testFailed = true; - - if (testFailed) { - if (!virtTestOOMActive() && - (flags & FLAG_EXPECT_FAILURE)) { - ret = 0; - VIR_TEST_DEBUG("Got expected error: %s\n", - virGetLastErrorMessage()); - virResetLastError(); - } - goto out; - } else if (flags & FLAG_EXPECT_FAILURE) { - VIR_TEST_DEBUG("qemuBuildCommandLine or qemuProcessStartValidate " - "should have failed\n"); - goto out; - } - - if (!virtTestOOMActive() && - (!!virGetLastError() != !!(flags & FLAG_EXPECT_ERROR))) { - if ((log = virtTestLogContentAndReset())) - VIR_TEST_DEBUG("\n%s", log); + domainLibDir, domainChannelTargetDir))) { + if (flags & FLAG_EXPECT_FAILURE) + goto ok; goto out; } @@ -391,15 +372,28 @@ static int testCompareXMLToArgvFiles(const char *xml, if (virtTestCompareToFile(actualargv, cmdline) < 0) goto out; + ret = 0; + ok: - if (!virtTestOOMActive() && - (flags & FLAG_EXPECT_ERROR)) { - /* need to suppress the errors */ + if (ret == 0 && + ((flags & FLAG_EXPECT_ERROR) | + (flags & FLAG_EXPECT_FAILURE))) { + ret = -1; + VIR_TEST_DEBUG("Error expected but there wasn't any.\n"); + goto out; + } + if (!virtTestOOMActive()) { + if (flags & FLAG_EXPECT_ERROR) { + if ((log = virtTestLogContentAndReset())) + VIR_TEST_DEBUG("Got expected error: \n%s", log); + } else if (flags & FLAG_EXPECT_FAILURE) { + VIR_TEST_DEBUG("Got expected failure: %s\n", + virGetLastErrorMessage()); + } virResetLastError(); + ret = 0; } - ret = 0; - out: VIR_FREE(log); VIR_FREE(actualargv); -- 2.7.2
On Tue, Mar 15, 2016 at 14:15:59 +0100, Pavel Hrdina wrote:
This removes the testFailed magic and makes the code more readable.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/qemuxml2argvtest.c | 62 ++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 34 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e15da37..150a50d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c ... @@ -391,15 +372,28 @@ static int testCompareXMLToArgvFiles(const char *xml, if (virtTestCompareToFile(actualargv, cmdline) < 0) goto out;
+ ret = 0; + ok: - if (!virtTestOOMActive() && - (flags & FLAG_EXPECT_ERROR)) { - /* need to suppress the errors */ + if (ret == 0 && + ((flags & FLAG_EXPECT_ERROR) | + (flags & FLAG_EXPECT_FAILURE))) {
Are you trying to optimize the code here or is it just a typo (| vs ||)? :-) In any case if (ret == 0 && (flags & FLAG_EXPECT_ERROR || flags & FLAG_EXPECT_FAILURE)) { or (if you prefer bit operations, which is uglier IMHO) if (ret == 0 && flags & (FLAG_EXPECT_ERROR | FLAG_EXPECT_FAILURE)) { would look a bit better.
+ ret = -1; + VIR_TEST_DEBUG("Error expected but there wasn't any.\n"); + goto out; + } + if (!virtTestOOMActive()) { + if (flags & FLAG_EXPECT_ERROR) { + if ((log = virtTestLogContentAndReset())) + VIR_TEST_DEBUG("Got expected error: \n%s", log); + } else if (flags & FLAG_EXPECT_FAILURE) { + VIR_TEST_DEBUG("Got expected failure: %s\n", + virGetLastErrorMessage());
Indentation is off a bit.
+ } virResetLastError(); + ret = 0; }
- ret = 0; - out: VIR_FREE(log); VIR_FREE(actualargv);
ACK once the nits are fixed. Jirka
On Tue, Mar 22, 2016 at 10:28:35AM +0100, Jiri Denemark wrote:
On Tue, Mar 15, 2016 at 14:15:59 +0100, Pavel Hrdina wrote:
This removes the testFailed magic and makes the code more readable.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/qemuxml2argvtest.c | 62 ++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 34 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e15da37..150a50d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c ... @@ -391,15 +372,28 @@ static int testCompareXMLToArgvFiles(const char *xml, if (virtTestCompareToFile(actualargv, cmdline) < 0) goto out;
+ ret = 0; + ok: - if (!virtTestOOMActive() && - (flags & FLAG_EXPECT_ERROR)) { - /* need to suppress the errors */ + if (ret == 0 && + ((flags & FLAG_EXPECT_ERROR) | + (flags & FLAG_EXPECT_FAILURE))) {
Are you trying to optimize the code here or is it just a typo (| vs ||)? :-) In any case
if (ret == 0 && (flags & FLAG_EXPECT_ERROR || flags & FLAG_EXPECT_FAILURE)) {
Yes :) it's a typo and I'll use this version.
or (if you prefer bit operations, which is uglier IMHO)
if (ret == 0 && flags & (FLAG_EXPECT_ERROR | FLAG_EXPECT_FAILURE)) {
would look a bit better.
+ ret = -1; + VIR_TEST_DEBUG("Error expected but there wasn't any.\n"); + goto out; + } + if (!virtTestOOMActive()) { + if (flags & FLAG_EXPECT_ERROR) { + if ((log = virtTestLogContentAndReset())) + VIR_TEST_DEBUG("Got expected error: \n%s", log); + } else if (flags & FLAG_EXPECT_FAILURE) { + VIR_TEST_DEBUG("Got expected failure: %s\n", + virGetLastErrorMessage());
Indentation is off a bit.
+ } virResetLastError(); + ret = 0; }
- ret = 0; - out: VIR_FREE(log); VIR_FREE(actualargv);
ACK once the nits are fixed.
Jirka
Fixed. Thanks, Pavel
Move all code that modifies only live XML to this function. The new VIR_QEMU_PROCESS_START_PRETEND flag will be used by qemuXMLToNative and qemuxml2argvtest later in order to reuse the same code as qemuProcessStart uses. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_migration.c | 4 ++ src/qemu/qemu_process.c | 159 +++++++++++++++++++++++++++------------------- src/qemu/qemu_process.h | 6 ++ 3 files changed, 102 insertions(+), 67 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 38fa81c..0b1d37b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3475,6 +3475,10 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto stopjob; dataFD[0] = -1; /* the FD is now owned by incoming */ + if (qemuProcessPrepareDomain(dconn, driver, vm, + VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) + goto stopjob; + rv = qemuProcessLaunch(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, incoming, NULL, VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 958fae3..df44fad 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4735,6 +4735,95 @@ qemuProcessSetupIOThreads(virDomainObjPtr vm) } +int +qemuProcessPrepareDomain(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int flags) +{ + int ret = -1; + size_t i; + char *nodeset = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + virCapsPtr caps; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0) + goto cleanup; + + if (VIR_ALLOC(priv->monConfig) < 0) + goto cleanup; + + VIR_DEBUG("Preparing monitor state"); + if (qemuProcessPrepareMonitorChr(priv->monConfig, priv->libDir) < 0) + goto cleanup; + + priv->monJSON = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON); + priv->monError = false; + priv->monStart = 0; + priv->gotShutdown = false; + + if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) { + /* If you are using a SecurityDriver with dynamic labelling, + then generate a security label for isolation */ + VIR_DEBUG("Generating domain security label (if required)"); + if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0) { + virDomainAuditSecurityLabel(vm, false); + goto cleanup; + } + virDomainAuditSecurityLabel(vm, true); + } + + /* "volume" type disk's source must be translated before + * cgroup and security setting. + */ + for (i = 0; i < vm->def->ndisks; i++) { + if (virStorageTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) + goto cleanup; + } + + /* Get the advisory nodeset from numad if 'placement' of + * either <vcpu> or <numatune> is 'auto'. + */ + if (virDomainDefNeedsPlacementAdvice(vm->def)) { + nodeset = virNumaGetAutoPlacementAdvice(virDomainDefGetVcpus(vm->def), + virDomainDefGetMemoryActual(vm->def)); + if (!nodeset) + goto cleanup; + + VIR_DEBUG("Nodeset returned from numad: %s", nodeset); + + if (virBitmapParse(nodeset, 0, &priv->autoNodeset, + VIR_DOMAIN_CPUMASK_LEN) < 0) + goto cleanup; + + if (!(priv->autoCpuset = virCapabilitiesGetCpusForNodemask(caps, + priv->autoNodeset))) + goto cleanup; + } + + /* + * Normally PCI addresses are assigned in the virDomainCreate + * or virDomainDefine methods. We might still need to assign + * some here to cope with the question of upgrades. Regardless + * we also need to populate the PCI address set cache for later + * use in hotplug + */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { + VIR_DEBUG("Assigning domain PCI addresses"); + if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnref(caps); + return ret; +} + + /** * qemuProcessLaunch: * @@ -4766,7 +4855,6 @@ qemuProcessLaunch(virConnectPtr conn, virCommandPtr cmd = NULL; struct qemuProcessHookData hookData; size_t i; - char *nodeset = NULL; virQEMUDriverConfigPtr cfg; virCapsPtr caps = NULL; unsigned int hostdev_flags = 0; @@ -4831,15 +4919,6 @@ qemuProcessLaunch(virConnectPtr conn, if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0) goto cleanup; - /* If you are using a SecurityDriver with dynamic labelling, - then generate a security label for isolation */ - VIR_DEBUG("Generating domain security label (if required)"); - if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0) { - virDomainAuditSecurityLabel(vm, false); - goto cleanup; - } - virDomainAuditSecurityLabel(vm, true); - if (vm->def->mem.nhugepages) { for (i = 0; i < cfg->nhugetlbfs; i++) { char *hugepagePath = qemuGetHugepagePath(&cfg->hugetlbfs[i]); @@ -4891,37 +4970,6 @@ qemuProcessLaunch(virConnectPtr conn, } } - if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0) - goto cleanup; - - /* Get the advisory nodeset from numad if 'placement' of - * either <vcpu> or <numatune> is 'auto'. - */ - if (virDomainDefNeedsPlacementAdvice(vm->def)) { - nodeset = virNumaGetAutoPlacementAdvice(virDomainDefGetVcpus(vm->def), - virDomainDefGetMemoryActual(vm->def)); - if (!nodeset) - goto cleanup; - - VIR_DEBUG("Nodeset returned from numad: %s", nodeset); - - if (virBitmapParse(nodeset, 0, &priv->autoNodeset, - VIR_DOMAIN_CPUMASK_LEN) < 0) - goto cleanup; - - if (!(priv->autoCpuset = virCapabilitiesGetCpusForNodemask(caps, - priv->autoNodeset))) - goto cleanup; - } - - /* "volume" type disk's source must be translated before - * cgroup and security setting. - */ - for (i = 0; i < vm->def->ndisks; i++) { - if (virStorageTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) - goto cleanup; - } - if (qemuDomainCheckDiskPresence(driver, vm, flags & VIR_QEMU_PROCESS_START_COLD) < 0) goto cleanup; @@ -4941,18 +4989,6 @@ qemuProcessLaunch(virConnectPtr conn, vm->def->id) < 0) goto cleanup; - if (VIR_ALLOC(priv->monConfig) < 0) - goto cleanup; - - VIR_DEBUG("Preparing monitor state"); - if (qemuProcessPrepareMonitorChr(priv->monConfig, priv->libDir) < 0) - goto cleanup; - - priv->monJSON = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON); - priv->monError = false; - priv->monStart = 0; - priv->gotShutdown = false; - VIR_FREE(priv->pidfile); if (!(priv->pidfile = virPidFileBuildPath(cfg->stateDir, vm->def->name))) { virReportSystemError(errno, @@ -4968,19 +5004,6 @@ qemuProcessLaunch(virConnectPtr conn, goto cleanup; } - /* - * Normally PCI addresses are assigned in the virDomainCreate - * or virDomainDefine methods. We might still need to assign - * some here to cope with the question of upgrades. Regardless - * we also need to populate the PCI address set cache for later - * use in hotplug - */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - VIR_DEBUG("Assigning domain PCI addresses"); - if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0) - goto cleanup; - } - VIR_DEBUG("Checking for any possible (non-fatal) issues"); /* @@ -5273,7 +5296,6 @@ qemuProcessLaunch(virConnectPtr conn, virObjectUnref(cfg); virObjectUnref(caps); VIR_FREE(nicindexes); - VIR_FREE(nodeset); return ret; } @@ -5366,6 +5388,9 @@ qemuProcessStart(virConnectPtr conn, goto stop; } + if (qemuProcessPrepareDomain(conn, driver, vm, flags) < 0) + goto stop; + if ((rv = qemuProcessLaunch(conn, driver, vm, asyncJob, incoming, snapshot, vmop, flags)) < 0) { if (rv == -2) diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index c21ed1c..ef5afa8 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -67,6 +67,7 @@ typedef enum { VIR_QEMU_PROCESS_START_COLD = 1 << 0, VIR_QEMU_PROCESS_START_PAUSED = 1 << 1, VIR_QEMU_PROCESS_START_AUTODESTROY = 1 << 2, + VIR_QEMU_PROCESS_START_PRETEND = 1 << 3, } qemuProcessStartFlags; int qemuProcessStart(virConnectPtr conn, @@ -92,6 +93,11 @@ int qemuProcessInit(virQEMUDriverPtr driver, bool migration, bool snap); +int qemuProcessPrepareDomain(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int flags); + int qemuProcessLaunch(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, -- 2.7.2
Move all code that modifies host system to this function. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_migration.c | 3 + src/qemu/qemu_process.c | 191 +++++++++++++++++++++++++--------------------- src/qemu/qemu_process.h | 4 + 3 files changed, 113 insertions(+), 85 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0b1d37b..f204855 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3479,6 +3479,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) goto stopjob; + if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0) + goto stopjob; + rv = qemuProcessLaunch(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, incoming, NULL, VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index df44fad..32efafb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4396,8 +4396,6 @@ qemuProcessInit(virQEMUDriverPtr driver, if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, true) < 0) goto stop; - if (qemuPrepareNVRAM(cfg, vm, migration) < 0) - goto stop; vm->def->id = qemuDriverAllocateID(driver); qemuDomainSetFakeReboot(driver, vm, false); @@ -4824,6 +4822,109 @@ qemuProcessPrepareDomain(virConnectPtr conn, } +int +qemuProcessPrepareHost(virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool incoming) +{ + int ret = -1; + unsigned int hostdev_flags = 0; + size_t i; + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + if (qemuPrepareNVRAM(cfg, vm, incoming) < 0) + goto cleanup; + + /* network devices must be "prepared" before hostdevs, because + * setting up a network device might create a new hostdev that + * will need to be setup. + */ + VIR_DEBUG("Preparing network devices"); + if (qemuProcessNetworkPrepareDevices(vm->def) < 0) + goto cleanup; + + /* Must be run before security labelling */ + VIR_DEBUG("Preparing host devices"); + if (!cfg->relaxedACS) + hostdev_flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; + if (!incoming) + hostdev_flags |= VIR_HOSTDEV_COLD_BOOT; + if (qemuHostdevPrepareDomainDevices(driver, vm->def, priv->qemuCaps, + hostdev_flags) < 0) + goto cleanup; + + VIR_DEBUG("Preparing chr devices"); + if (virDomainChrDefForeach(vm->def, + true, + qemuProcessPrepareChardevDevice, + NULL) < 0) + goto cleanup; + + if (vm->def->mem.nhugepages) { + for (i = 0; i < cfg->nhugetlbfs; i++) { + char *hugepagePath = qemuGetHugepagePath(&cfg->hugetlbfs[i]); + + if (!hugepagePath) + goto cleanup; + + if (virSecurityManagerSetHugepages(driver->securityManager, + vm->def, hugepagePath) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to set huge path in security driver")); + VIR_FREE(hugepagePath); + goto cleanup; + } + VIR_FREE(hugepagePath); + } + } + + /* Ensure no historical cgroup for this VM is lying around bogus + * settings */ + VIR_DEBUG("Ensuring no historical cgroup is lying around"); + qemuRemoveCgroup(vm); + + VIR_DEBUG("Setting up ports for graphics"); + if (qemuProcessSetupGraphics(driver, vm) < 0) + goto cleanup; + + if (virFileMakePath(cfg->logDir) < 0) { + virReportSystemError(errno, + _("cannot create log directory %s"), + cfg->logDir); + goto cleanup; + } + + VIR_FREE(priv->pidfile); + if (!(priv->pidfile = virPidFileBuildPath(cfg->stateDir, vm->def->name))) { + virReportSystemError(errno, + "%s", _("Failed to build pidfile path.")); + goto cleanup; + } + + if (unlink(priv->pidfile) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Cannot remove stale PID file %s"), + priv->pidfile); + goto cleanup; + } + + /* + * Create all per-domain directories in order to make sure domain + * with any possible seclabels can access it. + */ + if (qemuProcessMakeDir(driver, vm, priv->libDir) < 0 || + qemuProcessMakeDir(driver, vm, priv->channelTargetDir) < 0) + goto cleanup; + + ret = 0; + cleanup: + virObjectUnref(cfg); + return ret; +} + + /** * qemuProcessLaunch: * @@ -4857,7 +4958,6 @@ qemuProcessLaunch(virConnectPtr conn, size_t i; virQEMUDriverConfigPtr cfg; virCapsPtr caps = NULL; - unsigned int hostdev_flags = 0; size_t nnicindexes = 0; int *nicindexes = NULL; bool check_shmem = false; @@ -4890,69 +4990,10 @@ qemuProcessLaunch(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - /* network devices must be "prepared" before hostdevs, because - * setting up a network device might create a new hostdev that - * will need to be setup. - */ - VIR_DEBUG("Preparing network devices"); - if (qemuProcessNetworkPrepareDevices(vm->def) < 0) - goto cleanup; - - /* Must be run before security labelling */ - VIR_DEBUG("Preparing host devices"); - if (!cfg->relaxedACS) - hostdev_flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; - if (!incoming) - hostdev_flags |= VIR_HOSTDEV_COLD_BOOT; - if (qemuHostdevPrepareDomainDevices(driver, vm->def, priv->qemuCaps, - hostdev_flags) < 0) - goto cleanup; - - VIR_DEBUG("Preparing chr devices"); - if (virDomainChrDefForeach(vm->def, - true, - qemuProcessPrepareChardevDevice, - NULL) < 0) - goto cleanup; - VIR_DEBUG("Checking domain and device security labels"); if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0) goto cleanup; - if (vm->def->mem.nhugepages) { - for (i = 0; i < cfg->nhugetlbfs; i++) { - char *hugepagePath = qemuGetHugepagePath(&cfg->hugetlbfs[i]); - - if (!hugepagePath) - goto cleanup; - - if (virSecurityManagerSetHugepages(driver->securityManager, - vm->def, hugepagePath) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to set huge path in security driver")); - VIR_FREE(hugepagePath); - goto cleanup; - } - VIR_FREE(hugepagePath); - } - } - - /* Ensure no historical cgroup for this VM is lying around bogus - * settings */ - VIR_DEBUG("Ensuring no historical cgroup is lying around"); - qemuRemoveCgroup(vm); - - VIR_DEBUG("Setting up ports for graphics"); - if (qemuProcessSetupGraphics(driver, vm) < 0) - goto cleanup; - - if (virFileMakePath(cfg->logDir) < 0) { - virReportSystemError(errno, - _("cannot create log directory %s"), - cfg->logDir); - goto cleanup; - } - VIR_DEBUG("Creating domain log file"); if (!(logCtxt = qemuDomainLogContextNew(driver, vm, QEMU_DOMAIN_LOG_CONTEXT_MODE_START))) @@ -4989,21 +5030,6 @@ qemuProcessLaunch(virConnectPtr conn, vm->def->id) < 0) goto cleanup; - VIR_FREE(priv->pidfile); - if (!(priv->pidfile = virPidFileBuildPath(cfg->stateDir, vm->def->name))) { - virReportSystemError(errno, - "%s", _("Failed to build pidfile path.")); - goto cleanup; - } - - if (unlink(priv->pidfile) < 0 && - errno != ENOENT) { - virReportSystemError(errno, - _("Cannot remove stale PID file %s"), - priv->pidfile); - goto cleanup; - } - VIR_DEBUG("Checking for any possible (non-fatal) issues"); /* @@ -5066,14 +5092,6 @@ qemuProcessLaunch(virConnectPtr conn, if (incoming && incoming->fd != -1) virCommandPassFD(cmd, incoming->fd, 0); - /* - * Create all per-domain directories in order to make sure domain - * with any possible seclabels can access it. - */ - if (qemuProcessMakeDir(driver, vm, priv->libDir) < 0 || - qemuProcessMakeDir(driver, vm, priv->channelTargetDir) < 0) - goto cleanup; - /* now that we know it is about to start call the hook if present */ if (qemuProcessStartHook(driver, vm, VIR_HOOK_QEMU_OP_START, @@ -5391,6 +5409,9 @@ qemuProcessStart(virConnectPtr conn, if (qemuProcessPrepareDomain(conn, driver, vm, flags) < 0) goto stop; + if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0) + goto stop; + if ((rv = qemuProcessLaunch(conn, driver, vm, asyncJob, incoming, snapshot, vmop, flags)) < 0) { if (rv == -2) diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index ef5afa8..049c097 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -98,6 +98,10 @@ int qemuProcessPrepareDomain(virConnectPtr conn, virDomainObjPtr vm, unsigned int flags); +int qemuProcessPrepareHost(virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool incoming); + int qemuProcessLaunch(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, -- 2.7.2
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Move all code that modifies host system to this function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_migration.c | 3 + src/qemu/qemu_process.c | 191 +++++++++++++++++++++++++--------------------- src/qemu/qemu_process.h | 4 + 3 files changed, 113 insertions(+), 85 deletions(-)
ACK patch #4 and #5, but please add docstrings to the functions describing their purpose so we have more clear guidelines what checks should go where. Thanks, Cole
This change is required by following patches. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 33 ++++++++++++++++---------------- tests/qemuxml2argvtest.c | 49 +++++++++++++++++++++++++----------------------- 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0d6596..24fa3ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6949,9 +6949,9 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; - virDomainDefPtr def = NULL; virDomainChrSourceDef monConfig; virQEMUCapsPtr qemuCaps = NULL; + virDomainObjPtr vm = NULL; bool monitor_json = false; virCommandPtr cmd = NULL; char *ret = NULL; @@ -6977,22 +6977,21 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - def = virDomainDefParseString(xmlData, caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE | - VIR_DOMAIN_DEF_PARSE_ABI_UPDATE); - if (!def) + if (!(vm = virDomainObjNew(driver->xmlopt))) goto cleanup; - if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) + if (!(vm->def = virDomainDefParseString(xmlData, caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE))) goto cleanup; - if (qemuProcessStartValidate(def, qemuCaps, false, false) < 0) + if (qemuProcessStartValidate(driver, vm, qemuCaps, false, false) < 0) goto cleanup; /* Generate per-domain paths because we don't have the domain object */ if (qemuDomainSetPrivatePaths(&domainLibDir, &domainChannelTargetDir, cfg->libDir, cfg->channelTargetDir, - def->name, -1) < 0) + vm->def->name, -1) < 0) goto cleanup; /* Since we're just exporting args, we can't do bridge/network/direct @@ -7000,8 +6999,8 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, * directly. We convert those configs into generic 'ethernet' * config and assume the user has suitable 'ifup-qemu' scripts */ - for (i = 0; i < def->nnets; i++) { - virDomainNetDefPtr net = def->nets[i]; + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; int bootIndex = net->info.bootIndex; char *model = net->model; virMacAddr mac = net->mac; @@ -7085,18 +7084,18 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, monitor_json = virQEMUCapsGet(qemuCaps, QEMU_CAPS_MONITOR_JSON); - if (qemuProcessPrepareMonitorChr(&monConfig, def->name) < 0) + if (qemuProcessPrepareMonitorChr(&monConfig, vm->def->name) < 0) goto cleanup; - if (qemuAssignDeviceAliases(def, qemuCaps) < 0) + if (qemuAssignDeviceAliases(vm->def, qemuCaps) < 0) goto cleanup; - if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0) + if (qemuDomainAssignAddresses(vm->def, qemuCaps, NULL) < 0) goto cleanup; /* do fake auto-alloc of graphics ports, if such config is used */ - for (i = 0; i < def->ngraphics; ++i) { - virDomainGraphicsDefPtr graphics = def->graphics[i]; + for (i = 0; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && !graphics->data.vnc.socket && graphics->data.vnc.autoport) { graphics->data.vnc.port = 5900; @@ -7106,7 +7105,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, } } - if (!(cmd = qemuBuildCommandLine(conn, driver, NULL, def, + if (!(cmd = qemuBuildCommandLine(conn, driver, NULL, vm->def, &monConfig, monitor_json, qemuCaps, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, @@ -7123,7 +7122,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, cleanup: virObjectUnref(qemuCaps); virCommandFree(cmd); - virDomainDefFree(def); + virObjectUnref(vm); virObjectUnref(caps); virObjectUnref(cfg); VIR_FREE(domainLibDir); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 150a50d..135fbd5 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -258,7 +258,7 @@ static int testCompareXMLToArgvFiles(const char *xml, { char *actualargv = NULL; int ret = -1; - virDomainDefPtr vmdef = NULL; + virDomainObjPtr vm = NULL; virDomainChrSourceDef monitor_chr; virConnectPtr conn; char *log = NULL; @@ -276,24 +276,27 @@ static int testCompareXMLToArgvFiles(const char *xml, if (virBitmapParse("0-3", '\0', &nodeset, 4) < 0) goto out; - if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, - (VIR_DOMAIN_DEF_PARSE_INACTIVE | - parseFlags)))) { + if (!(vm = virDomainObjNew(driver.xmlopt))) + goto out; + + if (!(vm->def = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, + (VIR_DOMAIN_DEF_PARSE_INACTIVE | + parseFlags)))) { if (flags & FLAG_EXPECT_PARSE_ERROR) goto ok; goto out; } - if (!virDomainDefCheckABIStability(vmdef, vmdef)) { + if (!virDomainDefCheckABIStability(vm->def, vm->def)) { VIR_TEST_DEBUG("ABI stability check failed on %s", xml); goto out; } - vmdef->id = -1; + vm->def->id = -1; if (qemuDomainSetPrivatePaths(&domainLibDir, &domainChannelTargetDir, "/tmp/lib", "/tmp/channel", - vmdef->name, vmdef->id) < 0) + vm->def->name, vm->def->id) < 0) goto out; memset(&monitor_chr, 0, sizeof(monitor_chr)); @@ -305,16 +308,16 @@ static int testCompareXMLToArgvFiles(const char *xml, QEMU_CAPS_DEVICE, QEMU_CAPS_LAST); - if (STREQ(vmdef->os.machine, "pc") && - STREQ(vmdef->emulator, "/usr/bin/qemu-system-x86_64")) { - VIR_FREE(vmdef->os.machine); - if (VIR_STRDUP(vmdef->os.machine, "pc-0.11") < 0) + if (STREQ(vm->def->os.machine, "pc") && + STREQ(vm->def->emulator, "/usr/bin/qemu-system-x86_64")) { + VIR_FREE(vm->def->os.machine); + if (VIR_STRDUP(vm->def->os.machine, "pc-0.11") < 0) goto out; } - virQEMUCapsFilterByMachineType(extraFlags, vmdef->os.machine); + virQEMUCapsFilterByMachineType(extraFlags, vm->def->os.machine); - if (qemuDomainAssignAddresses(vmdef, extraFlags, NULL)) { + if (qemuDomainAssignAddresses(vm->def, extraFlags, NULL)) { if (flags & FLAG_EXPECT_ERROR) goto ok; goto out; @@ -324,16 +327,16 @@ static int testCompareXMLToArgvFiles(const char *xml, VIR_FREE(log); virResetLastError(); - if (vmdef->os.arch == VIR_ARCH_X86_64 || - vmdef->os.arch == VIR_ARCH_I686) { + if (vm->def->os.arch == VIR_ARCH_X86_64 || + vm->def->os.arch == VIR_ARCH_I686) { virQEMUCapsSet(extraFlags, QEMU_CAPS_PCI_MULTIBUS); } - if (qemuAssignDeviceAliases(vmdef, extraFlags) < 0) + if (qemuAssignDeviceAliases(vm->def, extraFlags) < 0) goto out; - for (i = 0; i < vmdef->nhostdevs; i++) { - virDomainHostdevDefPtr hostdev = vmdef->hostdevs[i]; + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && @@ -342,18 +345,18 @@ static int testCompareXMLToArgvFiles(const char *xml, } } - for (i = 0; i < vmdef->ndisks; i++) { - if (virStorageTranslateDiskSourcePool(conn, vmdef->disks[i]) < 0) + for (i = 0; i < vm->def->ndisks; i++) { + if (virStorageTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) goto out; } - if (qemuProcessStartValidate(vmdef, extraFlags, !!migrateURI, false) < 0) { + if (qemuProcessStartValidate(&driver, vm, extraFlags, !!migrateURI, false) < 0) { if (flags & FLAG_EXPECT_FAILURE) goto ok; goto out; } - if (!(cmd = qemuBuildCommandLine(conn, &driver, NULL, vmdef, &monitor_chr, + if (!(cmd = qemuBuildCommandLine(conn, &driver, NULL, vm->def, &monitor_chr, (flags & FLAG_JSON), extraFlags, migrateURI, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, @@ -398,7 +401,7 @@ static int testCompareXMLToArgvFiles(const char *xml, VIR_FREE(log); VIR_FREE(actualargv); virCommandFree(cmd); - virDomainDefFree(vmdef); + virObjectUnref(vm); virObjectUnref(conn); virBitmapFree(nodeset); VIR_FREE(domainLibDir); -- 2.7.2
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
This change is required by following patches.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 33 ++++++++++++++++---------------- tests/qemuxml2argvtest.c | 49 +++++++++++++++++++++++++----------------------- 2 files changed, 42 insertions(+), 40 deletions(-)
ACK - Cole
Move all code that checks host and domain. Do not check host if we use VIR_QEMU_PROCESS_START_PRETEND flag. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_process.c | 168 ++++++++++++++++++++++++---------------------- src/qemu/qemu_process.h | 9 ++- tests/qemuxml2argvtest.c | 5 +- 5 files changed, 103 insertions(+), 85 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 24fa3ec..1862c60 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6985,7 +6985,9 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE))) goto cleanup; - if (qemuProcessStartValidate(driver, vm, qemuCaps, false, false) < 0) + if (qemuProcessStartValidate(driver, vm, qemuCaps, false, false, + VIR_QEMU_PROCESS_START_COLD | + VIR_QEMU_PROCESS_START_PRETEND) < 0) goto cleanup; /* Generate per-domain paths because we don't have the domain object */ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f204855..8c7fd5c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3465,7 +3465,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, } if (qemuProcessInit(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, - true, false) < 0) + true, false, VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) goto stopjob; stopProcess = true; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 32efafb..6265cf5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4327,18 +4327,97 @@ qemuProcessMakeDir(virQEMUDriverPtr driver, * start of a VM. */ int -qemuProcessStartValidate(virDomainDefPtr def, +qemuProcessStartValidate(virQEMUDriverPtr driver, + virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, bool migration, - bool snapshot) + bool snapshot, + unsigned int flags) { - if (qemuValidateCpuCount(def, qemuCaps) < 0) + bool check_shmem = false; + size_t i; + + if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) { + if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) { + VIR_DEBUG("Checking for KVM availability"); + if (!virFileExists("/dev/kvm")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Domain requires KVM, but it is not available. " + "Check that virtualization is enabled in the " + "host BIOS, and host configuration is setup to " + "load the kvm modules.")); + return -1; + } + } + + if (qemuDomainCheckDiskPresence(driver, vm, + flags & VIR_QEMU_PROCESS_START_COLD) < 0) + return -1; + + VIR_DEBUG("Checking domain and device security labels"); + if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0) + return -1; + + } + + if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) return -1; if (!migration && !snapshot && - virDomainDefCheckDuplicateDiskInfo(def) < 0) + virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) return -1; + if (vm->def->mem.min_guarantee) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Parameter 'min_guarantee' " + "not supported by QEMU.")); + return -1; + } + + VIR_DEBUG("Checking for any possible (non-fatal) issues"); + + /* + * For vhost-user to work, the domain has to have some type of + * shared memory configured. We're not the proper ones to judge + * whether shared hugepages or shm are enough and will be in the + * future, so we'll just warn in case neither is configured. + * Moreover failing would give the false illusion that libvirt is + * really checking that everything works before running the domain + * and not only we are unable to do that, but it's also not our + * aim to do so. + */ + for (i = 0; i < vm->def->nnets; i++) { + if (virDomainNetGetActualType(vm->def->nets[i]) == + VIR_DOMAIN_NET_TYPE_VHOSTUSER) { + check_shmem = true; + break; + } + } + + if (check_shmem) { + bool shmem = vm->def->nshmems; + + /* + * This check is by no means complete. We merely check + * whether there are *some* hugepages enabled and *some* NUMA + * nodes with shared memory access. + */ + if (!shmem && vm->def->mem.nhugepages) { + for (i = 0; i < virDomainNumaGetNodeCount(vm->def->numa); i++) { + if (virDomainNumaGetNodeMemoryAccessMode(vm->def->numa, i) == + VIR_NUMA_MEM_ACCESS_SHARED) { + shmem = true; + break; + } + } + } + + if (!shmem) { + VIR_WARN("Detected vhost-user interface without any shared memory, " + "the interface might not be operational"); + } + } + return 0; } @@ -4356,7 +4435,8 @@ qemuProcessInit(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, bool migration, - bool snap) + bool snap, + unsigned int flags) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virCapsPtr caps = NULL; @@ -4385,7 +4465,8 @@ qemuProcessInit(virQEMUDriverPtr driver, vm->def->os.machine))) goto cleanup; - if (qemuProcessStartValidate(vm->def, priv->qemuCaps, migration, snap) < 0) + if (qemuProcessStartValidate(driver, vm, priv->qemuCaps, + migration, snap, flags) < 0) goto cleanup; /* Do this upfront, so any part of the startup process can add @@ -4955,12 +5036,10 @@ qemuProcessLaunch(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; virCommandPtr cmd = NULL; struct qemuProcessHookData hookData; - size_t i; virQEMUDriverConfigPtr cfg; virCapsPtr caps = NULL; size_t nnicindexes = 0; int *nicindexes = NULL; - bool check_shmem = false; VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d " "incoming.launchURI=%s incoming.deferredURI=%s " @@ -4990,38 +5069,12 @@ qemuProcessLaunch(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - VIR_DEBUG("Checking domain and device security labels"); - if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0) - goto cleanup; - VIR_DEBUG("Creating domain log file"); if (!(logCtxt = qemuDomainLogContextNew(driver, vm, QEMU_DOMAIN_LOG_CONTEXT_MODE_START))) goto cleanup; logfile = qemuDomainLogContextGetWriteFD(logCtxt); - if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) { - VIR_DEBUG("Checking for KVM availability"); - if (!virFileExists("/dev/kvm")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Domain requires KVM, but it is not available. " - "Check that virtualization is enabled in the host BIOS, " - "and host configuration is setup to load the kvm modules.")); - goto cleanup; - } - } - - if (qemuDomainCheckDiskPresence(driver, vm, - flags & VIR_QEMU_PROCESS_START_COLD) < 0) - goto cleanup; - - if (vm->def->mem.min_guarantee) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Parameter 'min_guarantee' " - "not supported by QEMU.")); - goto cleanup; - } - if (qemuDomainSetPrivatePaths(&priv->libDir, &priv->channelTargetDir, cfg->libDir, @@ -5030,50 +5083,6 @@ qemuProcessLaunch(virConnectPtr conn, vm->def->id) < 0) goto cleanup; - VIR_DEBUG("Checking for any possible (non-fatal) issues"); - - /* - * For vhost-user to work, the domain has to have some type of - * shared memory configured. We're not the proper ones to judge - * whether shared hugepages or shm are enough and will be in the - * future, so we'll just warn in case neither is configured. - * Moreover failing would give the false illusion that libvirt is - * really checking that everything works before running the domain - * and not only we are unable to do that, but it's also not our - * aim to do so. - */ - for (i = 0; i < vm->def->nnets; i++) { - if (virDomainNetGetActualType(vm->def->nets[i]) == - VIR_DOMAIN_NET_TYPE_VHOSTUSER) { - check_shmem = true; - break; - } - } - - if (check_shmem) { - bool shmem = vm->def->nshmems; - - /* - * This check is by no means complete. We merely check - * whether there are *some* hugepages enabled and *some* NUMA - * nodes with shared memory access. - */ - if (!shmem && vm->def->mem.nhugepages) { - for (i = 0; i < virDomainNumaGetNodeCount(vm->def->numa); i++) { - if (virDomainNumaGetNodeMemoryAccessMode(vm->def->numa, i) == - VIR_NUMA_MEM_ACCESS_SHARED) { - shmem = true; - break; - } - } - } - - if (!shmem) { - VIR_WARN("Detected vhost-user interface without any shared memory, " - "the interface might not be operational"); - } - } - VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(conn, driver, qemuDomainLogContextGetManager(logCtxt), @@ -5396,7 +5405,8 @@ qemuProcessStart(virConnectPtr conn, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY, cleanup); - if (qemuProcessInit(driver, vm, asyncJob, !!migrateFrom, !!snapshot) < 0) + if (qemuProcessInit(driver, vm, asyncJob, !!migrateFrom, + !!snapshot, flags) < 0) goto cleanup; if (migrateFrom) { diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 049c097..4820047 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -82,16 +82,19 @@ int qemuProcessStart(virConnectPtr conn, unsigned int flags); -int qemuProcessStartValidate(virDomainDefPtr def, +int qemuProcessStartValidate(virQEMUDriverPtr driver, + virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, bool migration, - bool snap); + bool snap, + unsigned int flags); int qemuProcessInit(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, bool migration, - bool snap); + bool snap, + unsigned int flags); int qemuProcessPrepareDomain(virConnectPtr conn, virQEMUDriverPtr driver, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 135fbd5..2c9dd1c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -350,7 +350,10 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } - if (qemuProcessStartValidate(&driver, vm, extraFlags, !!migrateURI, false) < 0) { + if (qemuProcessStartValidate(&driver, vm, extraFlags, + !!migrateURI, false, + VIR_QEMU_PROCESS_START_COLD | + VIR_QEMU_PROCESS_START_PRETEND) < 0) { if (flags & FLAG_EXPECT_FAILURE) goto ok; goto out; -- 2.7.2
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Move all code that checks host and domain. Do not check host if we use VIR_QEMU_PROCESS_START_PRETEND flag.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_process.c | 168 ++++++++++++++++++++++++---------------------- src/qemu/qemu_process.h | 9 ++- tests/qemuxml2argvtest.c | 5 +- 5 files changed, 103 insertions(+), 85 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 24fa3ec..1862c60 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6985,7 +6985,9 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE))) goto cleanup;
- if (qemuProcessStartValidate(driver, vm, qemuCaps, false, false) < 0) + if (qemuProcessStartValidate(driver, vm, qemuCaps, false, false, + VIR_QEMU_PROCESS_START_COLD | + VIR_QEMU_PROCESS_START_PRETEND) < 0) goto cleanup;
/* Generate per-domain paths because we don't have the domain object */ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f204855..8c7fd5c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3465,7 +3465,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, }
if (qemuProcessInit(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, - true, false) < 0) + true, false, VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) goto stopjob; stopProcess = true;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 32efafb..6265cf5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4327,18 +4327,97 @@ qemuProcessMakeDir(virQEMUDriverPtr driver, * start of a VM. */ int -qemuProcessStartValidate(virDomainDefPtr def, +qemuProcessStartValidate(virQEMUDriverPtr driver, + virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, bool migration, - bool snapshot) + bool snapshot, + unsigned int flags) { - if (qemuValidateCpuCount(def, qemuCaps) < 0) + bool check_shmem = false; + size_t i; + + if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) { + if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) { + VIR_DEBUG("Checking for KVM availability"); + if (!virFileExists("/dev/kvm")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Domain requires KVM, but it is not available. " + "Check that virtualization is enabled in the " + "host BIOS, and host configuration is setup to " + "load the kvm modules.")); + return -1; + } + } + + if (qemuDomainCheckDiskPresence(driver, vm, + flags & VIR_QEMU_PROCESS_START_COLD) < 0) + return -1; + + VIR_DEBUG("Checking domain and device security labels"); + if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0) + return -1; + + }
Can you add a comment before this block noting that anything which touches host state needs to be added to this block? Maybe even mention that test suite will break otherwise. Same could go for other uses of this flag in previous patches. ACK with that - Cole
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6265cf5..9894e3c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4491,6 +4491,14 @@ qemuProcessInit(virQEMUDriverPtr driver, VIR_HOOK_SUBOP_BEGIN) < 0) goto stop; + if (qemuDomainSetPrivatePaths(&priv->libDir, + &priv->channelTargetDir, + cfg->libDir, + cfg->channelTargetDir, + vm->def->name, + vm->def->id) < 0) + goto cleanup; + ret = 0; cleanup: @@ -5075,14 +5083,6 @@ qemuProcessLaunch(virConnectPtr conn, goto cleanup; logfile = qemuDomainLogContextGetWriteFD(logCtxt); - if (qemuDomainSetPrivatePaths(&priv->libDir, - &priv->channelTargetDir, - cfg->libDir, - cfg->channelTargetDir, - vm->def->name, - vm->def->id) < 0) - goto cleanup; - VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(conn, driver, qemuDomainLogContextGetManager(logCtxt), -- 2.7.2
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
ACK - Cole
This will skip few steps from qemuProcessStart in order to create only qemu CMD. Use a VIR_QEMU_PROCESS_START_PRETEND for all the qemuProcess* functions called by this one to not modify or check host. This new function will be used later on for XMLToNative API and also for qemuxml2argvtest to make sure that both API and test uses the same code as qemuProcessStart. We need also update qemuProcessInit to wrap few lines of code with check that VIR_QEMU_PROCESS_START_PRETEND that makes sense only for qemuProcessStart. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 73 +++++++++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_process.h | 7 +++++ 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9894e3c..dd85276 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4477,19 +4477,20 @@ qemuProcessInit(virQEMUDriverPtr driver, if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, true) < 0) goto stop; + if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) { + vm->def->id = qemuDriverAllocateID(driver); + qemuDomainSetFakeReboot(driver, vm, false); + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); - vm->def->id = qemuDriverAllocateID(driver); - qemuDomainSetFakeReboot(driver, vm, false); - virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); + if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); - if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) - driver->inhibitCallback(true, driver->inhibitOpaque); - - /* Run an early hook to set-up missing devices */ - if (qemuProcessStartHook(driver, vm, - VIR_HOOK_QEMU_OP_PREPARE, - VIR_HOOK_SUBOP_BEGIN) < 0) - goto stop; + /* Run an early hook to set-up missing devices */ + if (qemuProcessStartHook(driver, vm, + VIR_HOOK_QEMU_OP_PREPARE, + VIR_HOOK_SUBOP_BEGIN) < 0) + goto stop; + } if (qemuDomainSetPrivatePaths(&priv->libDir, &priv->channelTargetDir, @@ -5466,6 +5467,56 @@ qemuProcessStart(virConnectPtr conn, } +virCommandPtr +qemuProcessCreateCmd(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *migrateURI, + bool forceFips, + bool standalone, + unsigned int flags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virCommandPtr cmd = NULL; + + virCheckFlagsGoto(VIR_QEMU_PROCESS_START_COLD | + VIR_QEMU_PROCESS_START_PAUSED | + VIR_QEMU_PROCESS_START_AUTODESTROY, cleanup); + + flags |= VIR_QEMU_PROCESS_START_PRETEND; + + if (qemuProcessInit(driver, vm, QEMU_ASYNC_JOB_NONE, !!migrateURI, + false, flags) < 0) + goto cleanup; + + if (qemuProcessPrepareDomain(conn, driver, vm, flags) < 0) + goto cleanup; + + VIR_DEBUG("Building emulator command line"); + cmd = qemuBuildCommandLine(conn, + driver, + NULL, + vm->def, + priv->monConfig, + priv->monJSON, + priv->qemuCaps, + migrateURI, + NULL, + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + &buildCommandLineCallbacks, + standalone, + forceFips ? true : qemuCheckFips(), + priv->autoNodeset, + NULL, + NULL, + priv->libDir, + priv->channelTargetDir); + + cleanup: + return cmd; +} + + int qemuProcessKill(virDomainObjPtr vm, unsigned int flags) { diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 4820047..9e16aaf 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -81,6 +81,13 @@ int qemuProcessStart(virConnectPtr conn, virNetDevVPortProfileOp vmop, unsigned int flags); +virCommandPtr qemuProcessCreateCmd(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *migrateURI, + bool forceFips, + bool standalone, + unsigned int flags); int qemuProcessStartValidate(virQEMUDriverPtr driver, virDomainObjPtr vm, -- 2.7.2
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
This will skip few steps from qemuProcessStart in order to create only qemu CMD. Use a VIR_QEMU_PROCESS_START_PRETEND for all the qemuProcess* functions called by this one to not modify or check host.
This new function will be used later on for XMLToNative API and also for qemuxml2argvtest to make sure that both API and test uses the same code as qemuProcessStart.
We need also update qemuProcessInit to wrap few lines of code with check that VIR_QEMU_PROCESS_START_PRETEND that makes sense only for qemuProcessStart.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 73 +++++++++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_process.h | 7 +++++ 2 files changed, 69 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9894e3c..dd85276 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4477,19 +4477,20 @@ qemuProcessInit(virQEMUDriverPtr driver, if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, true) < 0) goto stop;
+ if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) { + vm->def->id = qemuDriverAllocateID(driver); + qemuDomainSetFakeReboot(driver, vm, false); + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
- vm->def->id = qemuDriverAllocateID(driver); - qemuDomainSetFakeReboot(driver, vm, false); - virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); + if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque);
- if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) - driver->inhibitCallback(true, driver->inhibitOpaque); - - /* Run an early hook to set-up missing devices */ - if (qemuProcessStartHook(driver, vm, - VIR_HOOK_QEMU_OP_PREPARE, - VIR_HOOK_SUBOP_BEGIN) < 0) - goto stop; + /* Run an early hook to set-up missing devices */ + if (qemuProcessStartHook(driver, vm, + VIR_HOOK_QEMU_OP_PREPARE, + VIR_HOOK_SUBOP_BEGIN) < 0) + goto stop; + }
if (qemuDomainSetPrivatePaths(&priv->libDir, &priv->channelTargetDir, @@ -5466,6 +5467,56 @@ qemuProcessStart(virConnectPtr conn, }
+virCommandPtr +qemuProcessCreateCmd(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *migrateURI, + bool forceFips, + bool standalone, + unsigned int flags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virCommandPtr cmd = NULL; + + virCheckFlagsGoto(VIR_QEMU_PROCESS_START_COLD | + VIR_QEMU_PROCESS_START_PAUSED | + VIR_QEMU_PROCESS_START_AUTODESTROY, cleanup); + + flags |= VIR_QEMU_PROCESS_START_PRETEND; + + if (qemuProcessInit(driver, vm, QEMU_ASYNC_JOB_NONE, !!migrateURI, + false, flags) < 0) + goto cleanup; + + if (qemuProcessPrepareDomain(conn, driver, vm, flags) < 0) + goto cleanup; + + VIR_DEBUG("Building emulator command line"); + cmd = qemuBuildCommandLine(conn, + driver, + NULL, + vm->def, + priv->monConfig, + priv->monJSON, + priv->qemuCaps, + migrateURI, + NULL, + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + &buildCommandLineCallbacks, + standalone, + forceFips ? true : qemuCheckFips(), + priv->autoNodeset, + NULL, + NULL, + priv->libDir, + priv->channelTargetDir); + + cleanup: + return cmd; +} + + int qemuProcessKill(virDomainObjPtr vm, unsigned int flags) { diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 4820047..9e16aaf 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -81,6 +81,13 @@ int qemuProcessStart(virConnectPtr conn, virNetDevVPortProfileOp vmop, unsigned int flags);
+virCommandPtr qemuProcessCreateCmd(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *migrateURI, + bool forceFips, + bool standalone, + unsigned int flags);
int qemuProcessStartValidate(virQEMUDriverPtr driver, virDomainObjPtr vm,
Can you call this qemuProcessCreatePretendCmd or something along those lines? The name is too generic, sounds like it's going to end up in the regular startup call chain ACK otherwise Thanks, Cole
Use qemuProcessCreateCmd instead duplicating required steps from qemuProcessStart. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 42 ++---------------------------------------- 1 file changed, 2 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1862c60..017b518 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6949,17 +6949,12 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; - virDomainChrSourceDef monConfig; - virQEMUCapsPtr qemuCaps = NULL; virDomainObjPtr vm = NULL; - bool monitor_json = false; virCommandPtr cmd = NULL; char *ret = NULL; size_t i; virQEMUDriverConfigPtr cfg; virCapsPtr caps = NULL; - char *domainLibDir = NULL; - char *domainChannelTargetDir = NULL; virCheckFlags(0, NULL); @@ -6985,17 +6980,6 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE))) goto cleanup; - if (qemuProcessStartValidate(driver, vm, qemuCaps, false, false, - VIR_QEMU_PROCESS_START_COLD | - VIR_QEMU_PROCESS_START_PRETEND) < 0) - goto cleanup; - - /* Generate per-domain paths because we don't have the domain object */ - if (qemuDomainSetPrivatePaths(&domainLibDir, &domainChannelTargetDir, - cfg->libDir, cfg->channelTargetDir, - vm->def->name, -1) < 0) - goto cleanup; - /* Since we're just exporting args, we can't do bridge/network/direct * setups, since libvirt will normally create TAP/macvtap devices * directly. We convert those configs into generic 'ethernet' @@ -7084,17 +7068,6 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, net->mac = mac; } - monitor_json = virQEMUCapsGet(qemuCaps, QEMU_CAPS_MONITOR_JSON); - - if (qemuProcessPrepareMonitorChr(&monConfig, vm->def->name) < 0) - goto cleanup; - - if (qemuAssignDeviceAliases(vm->def, qemuCaps) < 0) - goto cleanup; - - if (qemuDomainAssignAddresses(vm->def, qemuCaps, NULL) < 0) - goto cleanup; - /* do fake auto-alloc of graphics ports, if such config is used */ for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; @@ -7107,28 +7080,17 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, } } - if (!(cmd = qemuBuildCommandLine(conn, driver, NULL, vm->def, - &monConfig, monitor_json, qemuCaps, - NULL, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, - &buildCommandLineCallbacks, - true, - qemuCheckFips(), - NULL, NULL, NULL, - domainLibDir, - domainChannelTargetDir))) + if (!(cmd = qemuProcessCreateCmd(conn, driver, vm, NULL, false, true, + VIR_QEMU_PROCESS_START_COLD))) goto cleanup; ret = virCommandToString(cmd); cleanup: - virObjectUnref(qemuCaps); virCommandFree(cmd); virObjectUnref(vm); virObjectUnref(caps); virObjectUnref(cfg); - VIR_FREE(domainLibDir); - VIR_FREE(domainChannelTargetDir); return ret; } -- 2.7.2
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Use qemuProcessCreateCmd instead duplicating required steps from qemuProcessStart.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
ACK - Cole
Update testutilsqemu to overwrite libDir and channelTargetDir and set private paths using domain's privateData. This changes is required for following patch. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../qemuxml2argv-channel-virtio-unix.xml | 2 +- tests/qemuxml2argvtest.c | 18 +++++++----------- tests/testutilsqemu.c | 7 +++++++ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.xml index 405dff8..cef0445 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.xml @@ -33,7 +33,7 @@ <target type='virtio' name='ble'/> </channel> <channel type='unix'> - <source path='/tmp/domain-oldname/fdsa'/> + <source path='/tmp/channel/domain-oldname/fdsa'/> <target type='virtio' name='fdsa'/> </channel> <memballoon model='none'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2c9dd1c..43747e7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -265,8 +265,7 @@ static int testCompareXMLToArgvFiles(const char *xml, virCommandPtr cmd = NULL; size_t i; virBitmapPtr nodeset = NULL; - char *domainLibDir = NULL; - char *domainChannelTargetDir = NULL; + qemuDomainObjPrivatePtr priv = NULL; if (!(conn = virGetConnect())) goto out; @@ -286,6 +285,7 @@ static int testCompareXMLToArgvFiles(const char *xml, goto ok; goto out; } + priv = vm->privateData; if (!virDomainDefCheckABIStability(vm->def, vm->def)) { VIR_TEST_DEBUG("ABI stability check failed on %s", xml); @@ -294,13 +294,14 @@ static int testCompareXMLToArgvFiles(const char *xml, vm->def->id = -1; - if (qemuDomainSetPrivatePaths(&domainLibDir, &domainChannelTargetDir, - "/tmp/lib", "/tmp/channel", + if (qemuDomainSetPrivatePaths(&priv->libDir, &priv->channelTargetDir, + driver.config->libDir, + driver.config->channelTargetDir, vm->def->name, vm->def->id) < 0) goto out; memset(&monitor_chr, 0, sizeof(monitor_chr)); - if (qemuProcessPrepareMonitorChr(&monitor_chr, domainLibDir) < 0) + if (qemuProcessPrepareMonitorChr(&monitor_chr, priv->libDir) < 0) goto out; virQEMUCapsSetList(extraFlags, @@ -366,7 +367,7 @@ static int testCompareXMLToArgvFiles(const char *xml, &testCallbacks, false, (flags & FLAG_FIPS), nodeset, NULL, NULL, - domainLibDir, domainChannelTargetDir))) { + priv->libDir, priv->channelTargetDir))) { if (flags & FLAG_EXPECT_FAILURE) goto ok; goto out; @@ -407,8 +408,6 @@ static int testCompareXMLToArgvFiles(const char *xml, virObjectUnref(vm); virObjectUnref(conn); virBitmapFree(nodeset); - VIR_FREE(domainLibDir); - VIR_FREE(domainChannelTargetDir); return ret; } @@ -545,9 +544,6 @@ mymain(void) driver.config->spiceTLS = 1; if (VIR_STRDUP_QUIET(driver.config->spicePassword, "123456") < 0) return EXIT_FAILURE; - VIR_FREE(driver.config->channelTargetDir); - if (VIR_STRDUP_QUIET(driver.config->channelTargetDir, "/tmp") < 0) - return EXIT_FAILURE; # define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, \ parseFlags, ...) \ diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index ae69a18..1f854f5 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -564,6 +564,13 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (!driver->config) goto error; + /* Overwrite some default paths so it's consistent for tests. */ + VIR_FREE(driver->config->libDir); + VIR_FREE(driver->config->channelTargetDir); + if (VIR_STRDUP(driver->config->libDir, "/tmp/lib") < 0 || + VIR_STRDUP(driver->config->channelTargetDir, "/tmp/channel") < 0) + goto error; + driver->caps = testQemuCapsInit(); if (!driver->caps) goto error; -- 2.7.2
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Update testutilsqemu to overwrite libDir and channelTargetDir and set private paths using domain's privateData. This changes is required for following patch.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../qemuxml2argv-channel-virtio-unix.xml | 2 +- tests/qemuxml2argvtest.c | 18 +++++++----------- tests/testutilsqemu.c | 7 +++++++ 3 files changed, 15 insertions(+), 12 deletions(-)
ACK - Cole
Use qemuProcessCreateCmd instead duplicating required steps from qemuProcessStart. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/qemuxml2argvtest.c | 38 +++----------------------------------- 1 file changed, 3 insertions(+), 35 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 43747e7..ca7d6b7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -264,7 +264,6 @@ static int testCompareXMLToArgvFiles(const char *xml, char *log = NULL; virCommandPtr cmd = NULL; size_t i; - virBitmapPtr nodeset = NULL; qemuDomainObjPrivatePtr priv = NULL; if (!(conn = virGetConnect())) @@ -272,9 +271,6 @@ static int testCompareXMLToArgvFiles(const char *xml, conn->secretDriver = &fakeSecretDriver; conn->storageDriver = &fakeStorageDriver; - if (virBitmapParse("0-3", '\0', &nodeset, 4) < 0) - goto out; - if (!(vm = virDomainObjNew(driver.xmlopt))) goto out; @@ -294,11 +290,6 @@ static int testCompareXMLToArgvFiles(const char *xml, vm->def->id = -1; - if (qemuDomainSetPrivatePaths(&priv->libDir, &priv->channelTargetDir, - driver.config->libDir, - driver.config->channelTargetDir, - vm->def->name, vm->def->id) < 0) - goto out; memset(&monitor_chr, 0, sizeof(monitor_chr)); if (qemuProcessPrepareMonitorChr(&monitor_chr, priv->libDir) < 0) @@ -333,9 +324,6 @@ static int testCompareXMLToArgvFiles(const char *xml, virQEMUCapsSet(extraFlags, QEMU_CAPS_PCI_MULTIBUS); } - if (qemuAssignDeviceAliases(vm->def, extraFlags) < 0) - goto out; - for (i = 0; i < vm->def->nhostdevs; i++) { virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; @@ -346,28 +334,9 @@ static int testCompareXMLToArgvFiles(const char *xml, } } - for (i = 0; i < vm->def->ndisks; i++) { - if (virStorageTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) - goto out; - } - - if (qemuProcessStartValidate(&driver, vm, extraFlags, - !!migrateURI, false, - VIR_QEMU_PROCESS_START_COLD | - VIR_QEMU_PROCESS_START_PRETEND) < 0) { - if (flags & FLAG_EXPECT_FAILURE) - goto ok; - goto out; - } - - if (!(cmd = qemuBuildCommandLine(conn, &driver, NULL, vm->def, &monitor_chr, - (flags & FLAG_JSON), extraFlags, - migrateURI, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, - &testCallbacks, false, - (flags & FLAG_FIPS), - nodeset, NULL, NULL, - priv->libDir, priv->channelTargetDir))) { + if (!(cmd = qemuProcessCreateCmd(conn, &driver, vm, migrateURI, + (flags & FLAG_FIPS), false, + VIR_QEMU_PROCESS_START_COLD))) { if (flags & FLAG_EXPECT_FAILURE) goto ok; goto out; @@ -407,7 +376,6 @@ static int testCompareXMLToArgvFiles(const char *xml, virCommandFree(cmd); virObjectUnref(vm); virObjectUnref(conn); - virBitmapFree(nodeset); return ret; } -- 2.7.2
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Use qemuProcessCreateCmd instead duplicating required steps from qemuProcessStart.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/qemuxml2argvtest.c | 38 +++----------------------------------- 1 file changed, 3 insertions(+), 35 deletions(-)
ACK. Nice series! - Cole
On Tue, Mar 22, 2016 at 08:42:11AM -0400, Cole Robinson wrote:
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Use qemuProcessCreateCmd instead duplicating required steps from qemuProcessStart.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/qemuxml2argvtest.c | 38 +++----------------------------------- 1 file changed, 3 insertions(+), 35 deletions(-)
ACK. Nice series!
- Cole
Thanks, pushed now with the comments added and the function renamed. Pavel
Rebased version of this patch series: https://github.com/Antique/libvirt/tree/qemu-process
participants (3)
-
Cole Robinson -
Jiri Denemark -
Pavel Hrdina