10:13 a.m.
Martin Kletzander (3):
qemuhotplugtest: Allow testing of live data
qemu: Move channel path generation out of command creation
qemu: Generate channel target paths on hotplug as well
src/qemu/qemu_command.c | 25 ++--------
src/qemu/qemu_command.h | 5 +-
src/qemu/qemu_domain.c | 19 +++++++
src/qemu/qemu_domain.h | 4 ++
src/qemu/qemu_hotplug.c | 3 ++
src/qemu/qemu_process.c | 12 +++--
tests/qemuhotplugtest.c | 42 ++++++++++++----
.../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58 ++++++++++++++++++++++
.../qemuhotplug-hotplug-base+qemu-agent.xml | 58 ++++++++++++++++++++++
.../qemuhotplug-qemu-agent-detach.xml | 5 ++
.../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml | 5 ++
11 files changed, 197 insertions(+), 39 deletions(-)
create mode 100644
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
--
2.8.0
10:13 a.m.
New subject: [libvirt] [PATCH 1/3] qemuhotplugtest: Allow testing of live data
For now, the test was dumping an XML of inactive domain (well, setting
the id to '-1' to be precise) when checking the results. This patch
enables future additions to test the live XML output as well.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
tests/qemuhotplugtest.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 1eb2b6a881f2..2b0de94fb4a6 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -52,6 +52,7 @@ struct qemuHotplugTestData {
bool keep;
virDomainObjPtr vm;
bool deviceDeletedEvent;
+ bool live;
};
static int
@@ -177,12 +178,13 @@ static int
testQemuHotplugCheckResult(virDomainObjPtr vm,
const char *expected,
const char *expectedFile,
- bool fail)
+ bool fail, bool live)
{
char *actual;
int ret;
- vm->def->id = -1;
+ if (!live)
+ vm->def->id = -1;
actual = virDomainDefFormat(vm->def, driver.caps,
VIR_DOMAIN_DEF_FORMAT_SECURE);
if (!actual)
@@ -219,6 +221,7 @@ testQemuHotplug(const void *data)
const char *const *tmp;
bool fail = test->fail;
bool keep = test->keep;
+ bool live = test->live;
unsigned int device_parse_flags = 0;
virDomainObjPtr vm = NULL;
virDomainDeviceDefPtr dev = NULL;
@@ -300,14 +303,14 @@ testQemuHotplug(const void *data)
}
if (ret == 0 || fail)
ret = testQemuHotplugCheckResult(vm, result_xml,
- result_filename, fail);
+ result_filename, fail, live);
break;
case DETACH:
ret = testQemuHotplugDetach(vm, dev);
if (ret == 0 || fail)
ret = testQemuHotplugCheckResult(vm, domain_xml,
- domain_filename, fail);
+ domain_filename, fail, live);
break;
case UPDATE:
@@ -371,7 +374,7 @@ mymain(void)
/* wait only 100ms for DEVICE_DELETED event */
qemuDomainRemoveDeviceWaitTime = 100;
-#define DO_TEST(file, ACTION, dev, event, fial, kep, ...) \
+#define DO_TEST(file, ACTION, dev, event, fial, kep, liv, ...) \
do { \
const char *my_mon[] = { __VA_ARGS__, NULL}; \
const char *name = file " " #ACTION " " dev;
\
@@ -381,25 +384,29 @@ mymain(void)
data.fail = fial; \
data.mon = my_mon; \
data.keep = kep; \
+ data.live = liv; \
data.deviceDeletedEvent = event; \
if (virtTestRun(name, testQemuHotplug, &data) < 0) \
ret = -1; \
} while (0)
#define DO_TEST_ATTACH(file, dev, fial, kep, ...) \
- DO_TEST(file, ATTACH, dev, false, fial, kep, __VA_ARGS__)
+ DO_TEST(file, ATTACH, dev, false, fial, kep, false, __VA_ARGS__)
+
+#define DO_TEST_ATTACH_LIVE(file, dev, fial, kep, ...) \
+ DO_TEST(file, ATTACH, dev, false, fial, kep, true, __VA_ARGS__)
#define DO_TEST_DETACH(file, dev, fial, kep, ...) \
- DO_TEST(file, DETACH, dev, false, fial, kep, __VA_ARGS__)
+ DO_TEST(file, DETACH, dev, false, fial, kep, false, __VA_ARGS__)
#define DO_TEST_ATTACH_EVENT(file, dev, fial, kep, ...) \
- DO_TEST(file, ATTACH, dev, true, fial, kep, __VA_ARGS__)
+ DO_TEST(file, ATTACH, dev, true, fial, kep, false, __VA_ARGS__)
#define DO_TEST_DETACH_EVENT(file, dev, fial, kep, ...) \
- DO_TEST(file, DETACH, dev, true, fial, kep, __VA_ARGS__)
+ DO_TEST(file, DETACH, dev, true, fial, kep, false, __VA_ARGS__)
#define DO_TEST_UPDATE(file, dev, fial, kep, ...) \
- DO_TEST(file, UPDATE, dev, false, fial, kep, __VA_ARGS__)
+ DO_TEST(file, UPDATE, dev, false, fial, kep, false, __VA_ARGS__)
#define QMP_OK "{\"return\": {}}"
--
2.8.0
7:29 a.m.
New subject: [libvirt] [PATCH 1/3] qemuhotplugtest: Allow testing of live data
On 03/30/2016 11:13 AM, Martin Kletzander wrote:
For now, the test was dumping an XML of inactive domain (well,
setting
the id to '-1' to be precise) when checking the results. This patch
enables future additions to test the live XML output as well.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
tests/qemuhotplugtest.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 1eb2b6a881f2..2b0de94fb4a6 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -52,6 +52,7 @@ struct qemuHotplugTestData {
bool keep;
virDomainObjPtr vm;
bool deviceDeletedEvent;
+ bool live;
};
static int
@@ -177,12 +178,13 @@ static int
testQemuHotplugCheckResult(virDomainObjPtr vm,
const char *expected,
const char *expectedFile,
- bool fail)
+ bool fail, bool live)
{
char *actual;
int ret;
- vm->def->id = -1;
+ if (!live)
+ vm->def->id = -1;
actual = virDomainDefFormat(vm->def, driver.caps,
VIR_DOMAIN_DEF_FORMAT_SECURE);
if (!actual)
@@ -219,6 +221,7 @@ testQemuHotplug(const void *data)
const char *const *tmp;
bool fail = test->fail;
bool keep = test->keep;
+ bool live = test->live;
unsigned int device_parse_flags = 0;
virDomainObjPtr vm = NULL;
virDomainDeviceDefPtr dev = NULL;
@@ -300,14 +303,14 @@ testQemuHotplug(const void *data)
}
if (ret == 0 || fail)
ret = testQemuHotplugCheckResult(vm, result_xml,
- result_filename, fail);
+ result_filename, fail, live);
break;
case DETACH:
ret = testQemuHotplugDetach(vm, dev);
if (ret == 0 || fail)
ret = testQemuHotplugCheckResult(vm, domain_xml,
- domain_filename, fail);
+ domain_filename, fail, live);
break;
case UPDATE:
@@ -371,7 +374,7 @@ mymain(void)
/* wait only 100ms for DEVICE_DELETED event */
qemuDomainRemoveDeviceWaitTime = 100;
-#define DO_TEST(file, ACTION, dev, event, fial, kep, ...) \
+#define DO_TEST(file, ACTION, dev, event, fial, kep, liv, ...) \
"Typically" rather than misspelling "fail", "keep", or
"live" one would
use an underscore, such as "_fail", "_keep", and "_live" in
order to
represent a macro argument...
Not required for ACK, but nice nonetheless
John
do {
\
const char *my_mon[] = { __VA_ARGS__, NULL}; \
const char *name = file " " #ACTION " " dev;
\
@@ -381,25 +384,29 @@ mymain(void)
data.fail = fial; \
data.mon = my_mon; \
data.keep = kep; \
+ data.live = liv; \
data.deviceDeletedEvent = event; \
if (virtTestRun(name, testQemuHotplug, &data) < 0) \
ret = -1; \
} while (0)
#define DO_TEST_ATTACH(file, dev, fial, kep, ...) \
- DO_TEST(file, ATTACH, dev, false, fial, kep, __VA_ARGS__)
+ DO_TEST(file, ATTACH, dev, false, fial, kep, false, __VA_ARGS__)
+
+#define DO_TEST_ATTACH_LIVE(file, dev, fial, kep, ...) \
+ DO_TEST(file, ATTACH, dev, false, fial, kep, true, __VA_ARGS__)
#define DO_TEST_DETACH(file, dev, fial, kep, ...) \
- DO_TEST(file, DETACH, dev, false, fial, kep, __VA_ARGS__)
+ DO_TEST(file, DETACH, dev, false, fial, kep, false, __VA_ARGS__)
#define DO_TEST_ATTACH_EVENT(file, dev, fial, kep, ...) \
- DO_TEST(file, ATTACH, dev, true, fial, kep, __VA_ARGS__)
+ DO_TEST(file, ATTACH, dev, true, fial, kep, false, __VA_ARGS__)
#define DO_TEST_DETACH_EVENT(file, dev, fial, kep, ...) \
- DO_TEST(file, DETACH, dev, true, fial, kep, __VA_ARGS__)
+ DO_TEST(file, DETACH, dev, true, fial, kep, false, __VA_ARGS__)
#define DO_TEST_UPDATE(file, dev, fial, kep, ...) \
- DO_TEST(file, UPDATE, dev, false, fial, kep, __VA_ARGS__)
+ DO_TEST(file, UPDATE, dev, false, fial, kep, false, __VA_ARGS__)
#define QMP_OK "{\"return\": {}}"
5:43 a.m.
New subject: [libvirt] [PATCH 1/3] qemuhotplugtest: Allow testing of live data
On Thu, Mar 31, 2016 at 08:29:52AM -0400, John Ferlan wrote:
On 03/30/2016 11:13 AM, Martin Kletzander wrote:
> For now, the test was dumping an XML of inactive domain (well, setting
> the id to '-1' to be precise) when checking the results. This patch
> enables future additions to test the live XML output as well.
>
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> tests/qemuhotplugtest.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
> index 1eb2b6a881f2..2b0de94fb4a6 100644
> --- a/tests/qemuhotplugtest.c
> +++ b/tests/qemuhotplugtest.c
> @@ -371,7 +374,7 @@ mymain(void)
> /* wait only 100ms for DEVICE_DELETED event */
> qemuDomainRemoveDeviceWaitTime = 100;
>
> -#define DO_TEST(file, ACTION, dev, event, fial, kep, ...) \
> +#define DO_TEST(file, ACTION, dev, event, fial, kep, liv, ...) \
"Typically" rather than misspelling "fail", "keep", or
"live" one would
use an underscore, such as "_fail", "_keep", and "_live" in
order to
represent a macro argument...
Not required for ACK, but nice nonetheless
I didn't want to get to discussions about this so I followed what was
already used in this file (and bunch of others as well). If I were to
change this, it would go into separate patch anyway, so it doesn't make
sense to change in this one, I believe.
10:14 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: Move channel path generation out of command creation
Put it into separate function called qemuDomainPrepareChannel() and call
it from the new qemuProcessPrepareDomain().
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_command.c | 25 +++----------------------
src/qemu/qemu_command.h | 5 ++---
src/qemu/qemu_domain.c | 19 +++++++++++++++++++
src/qemu/qemu_domain.h | 4 ++++
src/qemu/qemu_process.c | 12 ++++++++----
5 files changed, 36 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 45c5398ad8b9..bb9357e41c3b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8468,8 +8468,7 @@ static int
qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
virCommandPtr cmd,
const virDomainDef *def,
- virQEMUCapsPtr qemuCaps,
- const char *domainChannelTargetDir)
+ virQEMUCapsPtr qemuCaps)
{
size_t i;
@@ -8508,22 +8507,6 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
return -1;
}
- /*
- * TODO: Refactor so that we generate this (and onther
- * things) somewhere else then where we are building the
- * command line.
- */
- if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
- !channel->source.data.nix.path) {
- if (virAsprintf(&channel->source.data.nix.path,
- "%s/%s", domainChannelTargetDir,
- channel->target.name ? channel->target.name
- : "unknown.sock") < 0)
- return -1;
-
- channel->source.data.nix.listen = true;
- }
-
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) &&
channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) {
/* spicevmc was originally introduced via a -device
@@ -9198,8 +9181,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virBitmapPtr nodeset,
size_t *nnicindexes,
int **nicindexes,
- const char *domainLibDir,
- const char *domainChannelTargetDir)
+ const char *domainLibDir)
{
size_t i;
char uuid[VIR_UUID_STRING_BUFLEN];
@@ -9340,8 +9322,7 @@ qemuBuildCommandLine(virConnectPtr conn,
if (qemuBuildParallelsCommandLine(logManager, cmd, def, qemuCaps) < 0)
goto error;
- if (qemuBuildChannelsCommandLine(logManager, cmd, def, qemuCaps,
- domainChannelTargetDir) < 0)
+ if (qemuBuildChannelsCommandLine(logManager, cmd, def, qemuCaps) < 0)
goto error;
if (qemuBuildConsoleCommandLine(logManager, cmd, def, qemuCaps) < 0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 7c13d459f8c5..60dc76c45bdb 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -74,10 +74,9 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
virBitmapPtr nodeset,
size_t *nnicindexes,
int **nicindexes,
- const char *domainLibDir,
- const char *domainChannelTargetDir)
+ const char *domainLibDir)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11)
- ATTRIBUTE_NONNULL(17) ATTRIBUTE_NONNULL(18);
+ ATTRIBUTE_NONNULL(17);
/* Generate '-device' string for chardev device */
int
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 403f01e75e46..645e1232d2f9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4639,3 +4639,22 @@ qemuDomainDiskByName(virDomainDefPtr def,
return ret;
}
+
+int
+qemuDomainPrepareChannel(virDomainChrDefPtr channel,
+ const char *domainChannelTargetDir)
+{
+ if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
+ channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
+ !channel->source.data.nix.path) {
+ if (virAsprintf(&channel->source.data.nix.path,
+ "%s/%s", domainChannelTargetDir,
+ channel->target.name ? channel->target.name
+ : "unknown.sock") < 0)
+ return -1;
+
+ channel->source.data.nix.listen = true;
+ }
+
+ return 0;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 02c6012c9c87..e893aa17cacd 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -560,4 +560,8 @@ int qemuDomainSetPrivatePaths(char **domainLibDir,
virDomainDiskDefPtr qemuDomainDiskByName(virDomainDefPtr def, const char *name);
+int
+qemuDomainPrepareChannel(virDomainChrDefPtr chr,
+ const char *domainChannelTargetDir);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 07b9df27397c..6a4fe17125b0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5008,6 +5008,12 @@ qemuProcessPrepareDomain(virConnectPtr conn,
goto cleanup;
}
+ for (i = 0; i < vm->def->nchannels; i++) {
+ if (qemuDomainPrepareChannel(vm->def->channels[i],
+ priv->channelTargetDir) < 0)
+ goto cleanup;
+ }
+
if (VIR_ALLOC(priv->monConfig) < 0)
goto cleanup;
@@ -5234,8 +5240,7 @@ qemuProcessLaunch(virConnectPtr conn,
qemuCheckFips(),
priv->autoNodeset,
&nnicindexes, &nicindexes,
- priv->libDir,
- priv->channelTargetDir)))
+ priv->libDir)))
goto cleanup;
if (incoming && incoming->fd != -1)
@@ -5656,8 +5661,7 @@ qemuProcessCreatePretendCmd(virConnectPtr conn,
priv->autoNodeset,
NULL,
NULL,
- priv->libDir,
- priv->channelTargetDir);
+ priv->libDir);
cleanup:
return cmd;
--
2.8.0
7:31 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: Move channel path generation out of command creation
On 03/30/2016 11:14 AM, Martin Kletzander wrote:
Put it into separate function called qemuDomainPrepareChannel() and
call
it from the new qemuProcessPrepareDomain().
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_command.c | 25 +++----------------------
src/qemu/qemu_command.h | 5 ++---
src/qemu/qemu_domain.c | 19 +++++++++++++++++++
src/qemu/qemu_domain.h | 4 ++++
src/qemu/qemu_process.c | 12 ++++++++----
5 files changed, 36 insertions(+), 29 deletions(-)
[...]
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 403f01e75e46..645e1232d2f9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4639,3 +4639,22 @@ qemuDomainDiskByName(virDomainDefPtr def,
return ret;
}
Two blank lines between functions.
ACK with that.
John
+
+int
+qemuDomainPrepareChannel(virDomainChrDefPtr channel,
+ const char *domainChannelTargetDir)
+{
+ if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
+ channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
+ !channel->source.data.nix.path) {
+ if (virAsprintf(&channel->source.data.nix.path,
+ "%s/%s", domainChannelTargetDir,
+ channel->target.name ? channel->target.name
+ : "unknown.sock") < 0)
+ return -1;
+
+ channel->source.data.nix.listen = true;
+ }
+
+ return 0;
+}
10:14 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Generate channel target paths on hotplug as well
Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9, qemu agent
channel cannot be plugged in because we won't generate its path
automatically. Let's not only fix that, but also add tests for it so
next time it's checked for.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1322210
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 ++
tests/qemuhotplugtest.c | 15 ++++++
.../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58 ++++++++++++++++++++++
.../qemuhotplug-hotplug-base+qemu-agent.xml | 58 ++++++++++++++++++++++
.../qemuhotplug-qemu-agent-detach.xml | 5 ++
.../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml | 5 ++
6 files changed, 144 insertions(+)
create mode 100644
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e82dbf5448dc..b7741e15d445 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1565,6 +1565,9 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
goto cleanup;
}
+ if (qemuDomainPrepareChannel(chr, priv->channelTargetDir) < 0)
+ goto cleanup;
+
if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
goto cleanup;
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 2b0de94fb4a6..384b7b9592b9 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -98,6 +98,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
(*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
+ if (qemuDomainSetPrivatePaths(&priv->libDir,
+ &priv->channelTargetDir,
+ "/var/lib/libvirt",
+ "/var/lib/libvirt/qemu/channel/target",
+ (*vm)->def->name,
+ (*vm)->def->id) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
return ret;
@@ -495,6 +503,13 @@ mymain(void)
"device_del", QMP_DEVICE_DELETED("scsi0-0-0-5")
QMP_OK,
"human-monitor-command", HMP(""));
+ DO_TEST_ATTACH_LIVE("hotplug-base", "qemu-agent", false, true,
+ "chardev-add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("hotplug-base", "qemu-agent-detach", false,
false,
+ "device_del", QMP_OK,
+ "chardev-remove", QMP_OK);
+
qemuTestDriverFree(&driver);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
new file mode 100644
index 000000000000..2c449f1d49cd
--- /dev/null
+++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
@@ -0,0 +1,58 @@
+<domain type='kvm' id='7'>
+ <name>hotplug</name>
+ <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
+ <memory unit='KiB'>4194304</memory>
+ <currentMemory unit='KiB'>4194304</currentMemory>
+ <vcpu placement='static'>4</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <controller type='usb' index='0'>
+ <alias name='usb'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <alias name='ide'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='scsi' index='0' model='virtio-scsi'>
+ <alias name='scsi0'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'>
+ <alias name='pci'/>
+ </controller>
+ <controller type='virtio-serial' index='0'>
+ <alias name='virtio-serial0'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ </controller>
+ <channel type='unix'>
+ <source mode='bind'
path='/var/lib/libvirt/qemu/channel/target/domain-7-hotplug/org.qemu.guest_agent.0'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ <alias name='channel0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
+ <input type='mouse' bus='ps2'>
+ <alias name='input0'/>
+ </input>
+ <input type='keyboard' bus='ps2'>
+ <alias name='input1'/>
+ </input>
+ <memballoon model='none'>
+ <alias name='balloon0'/>
+ </memballoon>
+ </devices>
+ <seclabel type='none' model='none'/>
+</domain>
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
new file mode 100644
index 000000000000..2c449f1d49cd
--- /dev/null
+++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
@@ -0,0 +1,58 @@
+<domain type='kvm' id='7'>
+ <name>hotplug</name>
+ <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
+ <memory unit='KiB'>4194304</memory>
+ <currentMemory unit='KiB'>4194304</currentMemory>
+ <vcpu placement='static'>4</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <controller type='usb' index='0'>
+ <alias name='usb'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <alias name='ide'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='scsi' index='0' model='virtio-scsi'>
+ <alias name='scsi0'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'>
+ <alias name='pci'/>
+ </controller>
+ <controller type='virtio-serial' index='0'>
+ <alias name='virtio-serial0'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ </controller>
+ <channel type='unix'>
+ <source mode='bind'
path='/var/lib/libvirt/qemu/channel/target/domain-7-hotplug/org.qemu.guest_agent.0'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ <alias name='channel0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
+ <input type='mouse' bus='ps2'>
+ <alias name='input0'/>
+ </input>
+ <input type='keyboard' bus='ps2'>
+ <alias name='input1'/>
+ </input>
+ <memballoon model='none'>
+ <alias name='balloon0'/>
+ </memballoon>
+ </devices>
+ <seclabel type='none' model='none'/>
+</domain>
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
b/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
new file mode 100644
index 000000000000..0c3c70a78e89
--- /dev/null
+++ b/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
@@ -0,0 +1,5 @@
+ <channel type='unix'>
+ <source mode='bind'
path='/var/lib/libvirt/qemu/channel/target/domain-7-hotplug/org.qemu.guest_agent.0'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
b/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
new file mode 100644
index 000000000000..f0e90dea3cc2
--- /dev/null
+++ b/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
@@ -0,0 +1,5 @@
+ <channel type='unix'>
+ <source mode='bind'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
--
2.8.0
7:35 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Generate channel target paths on hotplug as well
On 03/30/2016 11:14 AM, Martin Kletzander wrote:
Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9, qemu agent
channel cannot be plugged in because we won't generate its path
automatically. Let's not only fix that, but also add tests for it so
next time it's checked for.
Save some electrons, shorten the commit id
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1322210
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 ++
tests/qemuhotplugtest.c | 15 ++++++
.../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58 ++++++++++++++++++++++
.../qemuhotplug-hotplug-base+qemu-agent.xml | 58 ++++++++++++++++++++++
.../qemuhotplug-qemu-agent-detach.xml | 5 ++
.../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml | 5 ++
6 files changed, 144 insertions(+)
create mode 100644
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e82dbf5448dc..b7741e15d445 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1565,6 +1565,9 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
goto cleanup;
}
+ if (qemuDomainPrepareChannel(chr, priv->channelTargetDir) < 0)
+ goto cleanup;
+
if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
goto cleanup;
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 2b0de94fb4a6..384b7b9592b9 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -98,6 +98,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
(*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
+ if (qemuDomainSetPrivatePaths(&priv->libDir,
+ &priv->channelTargetDir,
+ "/var/lib/libvirt",
+ "/var/lib/libvirt/qemu/channel/target",
+ (*vm)->def->name,
+ (*vm)->def->id) < 0)
I believe this overwrites qemuTestDriverInit - since it's a test I'm not
concerned about the memory leak, just the processing consistency since
you're not really starting a guest, why change the paths?
While it's fresh in my mind (still) using /tmp/* in *DriverInit when I
was generating patches the domain master secret key file caused problems
if I actually tried to check for the existence, especially since
qemuProcessPrepareHost is where the qemuProcessMakeDir calls were made
to create the directory structure. Perhaps if the tests driver created
"tmp/*" paths rather than "/tmp/*" paths that'd work, but is more
or
less unrelated.
I'll wait for your thoughts on this before an official ACK -
John
+ goto cleanup;
+
ret = 0;
cleanup:
return ret;
@@ -495,6 +503,13 @@ mymain(void)
"device_del", QMP_DEVICE_DELETED("scsi0-0-0-5")
QMP_OK,
"human-monitor-command", HMP(""));
+ DO_TEST_ATTACH_LIVE("hotplug-base", "qemu-agent", false, true,
+ "chardev-add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("hotplug-base", "qemu-agent-detach", false,
false,
+ "device_del", QMP_OK,
+ "chardev-remove", QMP_OK);
+
qemuTestDriverFree(&driver);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
[...]
7:43 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Generate channel target paths on hotplug as well
On Thu, Mar 31, 2016 at 08:35:43AM -0400, John Ferlan wrote:
On 03/30/2016 11:14 AM, Martin Kletzander wrote:
> Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9, qemu agent
> channel cannot be plugged in because we won't generate its path
> automatically. Let's not only fix that, but also add tests for it so
> next time it's checked for.
>
Save some electrons, shorten the commit id
I did that, but I wonder whether you haven't used more electrons by
mentioning that than I just shaved off =D
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1322210
>
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> src/qemu/qemu_hotplug.c | 3 ++
> tests/qemuhotplugtest.c | 15 ++++++
> .../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58 ++++++++++++++++++++++
> .../qemuhotplug-hotplug-base+qemu-agent.xml | 58 ++++++++++++++++++++++
> .../qemuhotplug-qemu-agent-detach.xml | 5 ++
> .../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml | 5 ++
> 6 files changed, 144 insertions(+)
> create mode 100644
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
> create mode 100644
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
> create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
> create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
>
[...]
> diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
> index 2b0de94fb4a6..384b7b9592b9 100644
> --- a/tests/qemuhotplugtest.c
> +++ b/tests/qemuhotplugtest.c
> @@ -98,6 +98,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
>
> (*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
>
> + if (qemuDomainSetPrivatePaths(&priv->libDir,
> + &priv->channelTargetDir,
> + "/var/lib/libvirt",
> + "/var/lib/libvirt/qemu/channel/target",
> + (*vm)->def->name,
> + (*vm)->def->id) < 0)
I believe this overwrites qemuTestDriverInit - since it's a test I'm not
concerned about the memory leak, just the processing consistency since
you're not really starting a guest, why change the paths?
Well, I just needed to initialize it to some string. I can change it to
/tmp without any problems. However it is not used anywhere to access
anything and we would need to change a lot of tests that make sense
currently (as they generate those values -- one of them is even checking
that it generates that particular value).
Moreover, it should not introduce any leak as it is set only if the
values are empty.
While it's fresh in my mind (still) using /tmp/* in *DriverInit
when I
was generating patches the domain master secret key file caused problems
if I actually tried to check for the existence, especially since
qemuProcessPrepareHost is where the qemuProcessMakeDir calls were made
to create the directory structure. Perhaps if the tests driver created
"tmp/*" paths rather than "/tmp/*" paths that'd work, but is more
or
less unrelated.
Oh, that reminded me that I should figure out why I can't start any
machine since those patches went in...
Anyway, I'm not aware about the fact that qemuProcessPrepareHost() is
called in tests. And if it is, that's the problem by itself.
I'll wait for your thoughts on this before an official ACK -
John
6:25 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Generate channel target paths on hotplug as well
On 04/12/2016 08:43 AM, Martin Kletzander wrote:
On Thu, Mar 31, 2016 at 08:35:43AM -0400, John Ferlan wrote:
> On 03/30/2016 11:14 AM, Martin Kletzander wrote:
>> Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9, qemu agent
>> channel cannot be plugged in because we won't generate its path
>> automatically. Let's not only fix that, but also add tests for it so
>> next time it's checked for.
>>
>
> Save some electrons, shorten the commit id
>
I did that, but I wonder whether you haven't used more electrons by
mentioning that than I just shaved off =D
touche
>> Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1322210
>>
>> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
>> ---
>> src/qemu/qemu_hotplug.c | 3 ++
>> tests/qemuhotplugtest.c | 15 ++++++
>> .../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58
>> ++++++++++++++++++++++
>> .../qemuhotplug-hotplug-base+qemu-agent.xml | 58
>> ++++++++++++++++++++++
>> .../qemuhotplug-qemu-agent-detach.xml | 5 ++
>> .../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml | 5 ++
>> 6 files changed, 144 insertions(+)
>> create mode 100644
>> tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
>> create mode 100644
>> tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
>> create mode 100644
>> tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
>> create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
>>
[...]
>> diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
>> index 2b0de94fb4a6..384b7b9592b9 100644
>> --- a/tests/qemuhotplugtest.c
>> +++ b/tests/qemuhotplugtest.c
>> @@ -98,6 +98,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr
>> xmlopt,
>>
>> (*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
>>
>> + if (qemuDomainSetPrivatePaths(&priv->libDir,
>> + &priv->channelTargetDir,
>> + "/var/lib/libvirt",
>> +
>> "/var/lib/libvirt/qemu/channel/target",
>> + (*vm)->def->name,
>> + (*vm)->def->id) < 0)
>
> I believe this overwrites qemuTestDriverInit - since it's a test I'm not
> concerned about the memory leak, just the processing consistency since
> you're not really starting a guest, why change the paths?
>
Well, I just needed to initialize it to some string. I can change it to
/tmp without any problems. However it is not used anywhere to access
anything and we would need to change a lot of tests that make sense
currently (as they generate those values -- one of them is even checking
that it generates that particular value).
Moreover, it should not introduce any leak as it is set only if the
values are empty.
oh right - although I imagine even this changes since commit id
'1893b6df1' altered the API. I'm sure you know that already though ;-)
ACK for this though
John
> While it's fresh in my mind (still) using /tmp/* in
*DriverInit when I
> was generating patches the domain master secret key file caused problems
> if I actually tried to check for the existence, especially since
> qemuProcessPrepareHost is where the qemuProcessMakeDir calls were made
> to create the directory structure. Perhaps if the tests driver created
> "tmp/*" paths rather than "/tmp/*" paths that'd work, but is
more or
> less unrelated.
>
Oh, that reminded me that I should figure out why I can't start any
machine since those patches went in...
Anyway, I'm not aware about the fact that qemuProcessPrepareHost() is
called in tests. And if it is, that's the problem by itself.
8:06 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Generate channel target paths on hotplug as well
On Wed, Apr 13, 2016 at 07:25:17AM -0400, John Ferlan wrote:
On 04/12/2016 08:43 AM, Martin Kletzander wrote:
> On Thu, Mar 31, 2016 at 08:35:43AM -0400, John Ferlan wrote:
>> On 03/30/2016 11:14 AM, Martin Kletzander wrote:
>>> Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9, qemu agent
>>> channel cannot be plugged in because we won't generate its path
>>> automatically. Let's not only fix that, but also add tests for it so
>>> next time it's checked for.
>>>
>>
>> Save some electrons, shorten the commit id
>>
>
> I did that, but I wonder whether you haven't used more electrons by
> mentioning that than I just shaved off =D
>
touche
>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1322210
>>>
>>> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
>>> ---
>>> src/qemu/qemu_hotplug.c | 3 ++
>>> tests/qemuhotplugtest.c | 15 ++++++
>>> .../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58
>>> ++++++++++++++++++++++
>>> .../qemuhotplug-hotplug-base+qemu-agent.xml | 58
>>> ++++++++++++++++++++++
>>> .../qemuhotplug-qemu-agent-detach.xml | 5 ++
>>> .../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml | 5 ++
>>> 6 files changed, 144 insertions(+)
>>> create mode 100644
>>> tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
>>> create mode 100644
>>> tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
>>> create mode 100644
>>> tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
>>> create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
>>>
>
> [...]
>
>>> diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
>>> index 2b0de94fb4a6..384b7b9592b9 100644
>>> --- a/tests/qemuhotplugtest.c
>>> +++ b/tests/qemuhotplugtest.c
>>> @@ -98,6 +98,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr
>>> xmlopt,
>>>
>>> (*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
>>>
>>> + if (qemuDomainSetPrivatePaths(&priv->libDir,
>>> + &priv->channelTargetDir,
>>> + "/var/lib/libvirt",
>>> +
>>> "/var/lib/libvirt/qemu/channel/target",
>>> + (*vm)->def->name,
>>> + (*vm)->def->id) < 0)
>>
>> I believe this overwrites qemuTestDriverInit - since it's a test I'm not
>> concerned about the memory leak, just the processing consistency since
>> you're not really starting a guest, why change the paths?
>>
>
>
> Well, I just needed to initialize it to some string. I can change it to
> /tmp without any problems. However it is not used anywhere to access
> anything and we would need to change a lot of tests that make sense
> currently (as they generate those values -- one of them is even checking
> that it generates that particular value).
>
> Moreover, it should not introduce any leak as it is set only if the
> values are empty.
>
oh right - although I imagine even this changes since commit id
'1893b6df1' altered the API. I'm sure you know that already though ;-)
That commit didn't change it, fortunately =)
ACK for this though
Thanks for the review. Just to make sure, I can push the whole series?
John
>> While it's fresh in my mind (still) using /tmp/* in *DriverInit when I
>> was generating patches the domain master secret key file caused problems
>> if I actually tried to check for the existence, especially since
>> qemuProcessPrepareHost is where the qemuProcessMakeDir calls were made
>> to create the directory structure. Perhaps if the tests driver created
>> "tmp/*" paths rather than "/tmp/*" paths that'd work, but
is more or
>> less unrelated.
>>
>
> Oh, that reminded me that I should figure out why I can't start any
> machine since those patches went in...
>
> Anyway, I'm not aware about the fact that qemuProcessPrepareHost() is
> called in tests. And if it is, that's the problem by itself.
>
3146
days inactive
3160
days old
10 comments
2 participants
participants (2)
-
John Ferlan -
Martin Kletzander