8:15 a.m.
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
8:15 a.m.
New subject: [libvirt] [PATCH 01/12] qemu_process: check for correct return value while starting domain
Function qemuProcessLaunch returns '-2' in case there was an error and
we need to cleanup labels.
Signed-off-by: Pavel Hrdina <phrdina(a)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
3:21 a.m.
New subject: [libvirt] [PATCH 01/12] qemu_process: check for correct return value while starting domain
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(a)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
8:15 a.m.
New subject: [libvirt] [PATCH 02/12] nvram: generate it's path in qemuDomainDefPostParse
The postParse callback is the correct place to generate default values
that should be present in offline XML.
Signed-off-by: Pavel Hrdina <phrdina(a)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
4:06 a.m.
New subject: [libvirt] [PATCH 02/12] nvram: generate it's path in qemuDomainDefPostParse
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(a)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
7:03 a.m.
New subject: [libvirt] [PATCH 02/12] nvram: generate it's path in qemuDomainDefPostParse
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(a)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
8:15 a.m.
New subject: [libvirt] [PATCH 03/12] tests: cleanup qemuxml2argvtest
This removes the testFailed magic and makes the code more readable.
Signed-off-by: Pavel Hrdina <phrdina(a)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
4:28 a.m.
New subject: [libvirt] [PATCH 03/12] tests: cleanup qemuxml2argvtest
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(a)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
7:04 a.m.
New subject: [libvirt] [PATCH 03/12] tests: cleanup qemuxml2argvtest
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(a)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
8:16 a.m.
New subject: [libvirt] [PATCH 04/12] qemu_process: introduce qemuProcessPrepareDomain
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(a)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
8:16 a.m.
New subject: [libvirt] [PATCH 05/12] qemu_process: introduce qemuProcessPrepareHost
Move all code that modifies host system to this function.
Signed-off-by: Pavel Hrdina <phrdina(a)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
7:19 a.m.
New subject: [libvirt] [PATCH 05/12] qemu_process: introduce qemuProcessPrepareHost
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(a)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
8:16 a.m.
New subject: [libvirt] [PATCH 06/12] qemu: update callers of qemuProcessStartValidate to use virDomainObjPtr
This change is required by following patches.
Signed-off-by: Pavel Hrdina <phrdina(a)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
7:20 a.m.
New subject: [libvirt] [PATCH 06/12] qemu: update callers of qemuProcessStartValidate to use virDomainObjPtr
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
This change is required by following patches.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 33 ++++++++++++++++----------------
tests/qemuxml2argvtest.c | 49 +++++++++++++++++++++++++-----------------------
2 files changed, 42 insertions(+), 40 deletions(-)
ACK
- Cole
8:16 a.m.
New subject: [libvirt] [PATCH 07/12] qemu_process: move checks to qemuProcessStartValidate
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(a)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
7:30 a.m.
New subject: [libvirt] [PATCH 07/12] qemu_process: move checks to qemuProcessStartValidate
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(a)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
8:16 a.m.
New subject: [libvirt] [PATCH 08/12] qemu_process: move qemuDomainSetPrivatePaths to qemuProcessInit
Signed-off-by: Pavel Hrdina <phrdina(a)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
7:31 a.m.
New subject: [libvirt] [PATCH 08/12] qemu_process: move qemuDomainSetPrivatePaths to qemuProcessInit
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_process.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
ACK
- Cole
8:16 a.m.
New subject: [libvirt] [PATCH 09/12] qemu_process: introduce qemuProcessCreateCmd
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(a)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
7:36 a.m.
New subject: [libvirt] [PATCH 09/12] qemu_process: introduce qemuProcessCreateCmd
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(a)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
8:16 a.m.
New subject: [libvirt] [PATCH 10/12] qemu_driver: cleanup qemuConnectDomainXMLToNative
Use qemuProcessCreateCmd instead duplicating required steps from
qemuProcessStart.
Signed-off-by: Pavel Hrdina <phrdina(a)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
7:37 a.m.
New subject: [libvirt] [PATCH 10/12] qemu_driver: cleanup qemuConnectDomainXMLToNative
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Use qemuProcessCreateCmd instead duplicating required steps from
qemuProcessStart.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
ACK
- Cole
8:16 a.m.
New subject: [libvirt] [PATCH 11/12] qemuxml2argvtest: use driver.config and priv for qemuDomainSetPrivatePaths
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(a)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
7:38 a.m.
New subject: [libvirt] [PATCH 11/12] qemuxml2argvtest: use driver.config and priv for qemuDomainSetPrivatePaths
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(a)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
8:16 a.m.
New subject: [libvirt] [PATCH 12/12] qemuxml2argvtest: cleanup test
Use qemuProcessCreateCmd instead duplicating required steps from
qemuProcessStart.
Signed-off-by: Pavel Hrdina <phrdina(a)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
7:42 a.m.
New subject: [libvirt] [PATCH 12/12] qemuxml2argvtest: cleanup test
On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
Use qemuProcessCreateCmd instead duplicating required steps from
qemuProcessStart.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
tests/qemuxml2argvtest.c | 38 +++-----------------------------------
1 file changed, 3 insertions(+), 35 deletions(-)
ACK. Nice series!
- Cole
9:22 a.m.
New subject: [libvirt] [PATCH 12/12] qemuxml2argvtest: cleanup test
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(a)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
8:14 a.m.
Rebased version of this patch series:
https://github.com/Antique/libvirt/tree/qemu-process
3197
days inactive
3204
days old
27 comments
3 participants
participants (3)
-
Cole Robinson -
Jiri Denemark -
Pavel Hrdina