At 10/22/2011 12:48 AM, Eric Blake Write:
> Rather than making all clients of monitor commands that are JSON-only
> check whether yajl support was compiled in, it is simpler to just
> avoid setting the capability bit up front if we can't use the capability.
>
> * src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Only set
> capability bit if we also have yajl library to use it.
> * src/qemu/qemu_driver.c (qemuDomainReboot): Drop #ifdefs.
> * src/qemu/qemu_process.c (qemuProcessStart): Likewise.
> * tests/qemuhelptest.c (testHelpStrParsing): Pass test even
> without yajl.
I have installed yajl-devel, so I did not find this problem when
I review and test V1.
> * tests/qemuxml2argvtest.c (mymain): Simplify use of json flag.
> * tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-*.args:
> Update expected results to match.
> ---
> src/qemu/qemu_capabilities.c | 2 ++
> src/qemu/qemu_driver.c | 6 ------
> src/qemu/qemu_process.c | 2 --
> tests/qemuhelptest.c | 5 +++++
> ...uxml2argv-disk-drive-error-policy-enospace.args | 3 ++-
> .../qemuxml2argv-disk-drive-error-policy-stop.args | 3 ++-
> ...gv-disk-drive-error-policy-wreport-rignore.args | 3 ++-
> tests/qemuxml2argvtest.c | 10 ++++------
> 8 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 5f0356c..b4ab55b 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1060,8 +1060,10 @@ qemuCapsComputeCmdFlags(const char *help,
> * two features. The benefits of JSON mode now outweigh
> * the downside.
> */
> +#if HAVE_YAJL
> if (version >= 13000)
> qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON);
> +#endif
>
> if (version >= 13000)
> qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6b65716..c9762ba 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1545,9 +1545,7 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) {
> struct qemud_driver *driver = dom->conn->privateData;
> virDomainObjPtr vm;
> int ret = -1;
> -#if HAVE_YAJL
> qemuDomainObjPrivatePtr priv;
> -#endif
>
> virCheckFlags(0, -1);
>
> @@ -1563,7 +1561,6 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) {
> goto cleanup;
> }
>
> -#if HAVE_YAJL
> priv = vm->privateData;
>
> if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) {
> @@ -1593,12 +1590,9 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) {
> if (qemuDomainObjEndJob(driver, vm) == 0)
> vm = NULL;
> } else {
> -#endif
> qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("Reboot is not supported without the JSON monitor"));
> -#if HAVE_YAJL
> }
> -#endif
>
> cleanup:
> if (vm)
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a7fe86c..cc2395f 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2962,11 +2962,9 @@ int qemuProcessStart(virConnectPtr conn,
> if (qemuProcessPrepareMonitorChr(driver, priv->monConfig, vm->def->name) < 0)
> goto cleanup;
>
> -#if HAVE_YAJL
> if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON))
> priv->monJSON = 1;
> else
> -#endif
> priv->monJSON = 0;
>
> priv->monError = false;
> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
> index fcee41f..18269a2 100644
> --- a/tests/qemuhelptest.c
> +++ b/tests/qemuhelptest.c
> @@ -56,6 +56,11 @@ static int testHelpStrParsing(const void *data)
> &version, &is_kvm, &kvm_version) == -1)
> goto cleanup;
>
> +#ifndef HAVE_YAJL
> + if (qemuCapsGet(info->flags, QEMU_CAPS_MONITOR_JSON))
> + qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON);
> +#endif
make syntax-check will fail here.
It should be:
# ifndef HAVE_YAJL
...
# endif
> +
> if (qemuCapsGet(info->flags, QEMU_CAPS_DEVICE)) {
> VIR_FREE(path);
> VIR_FREE(help);
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.args
> index 981d410..0fc6ccc 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.args
> @@ -1,5 +1,6 @@
> LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
> -pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
> +pc -m 214 -smp 1 -nographic \
> +-monitor control,unix:/tmp/test-monitor,server,nowait \
> -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,\
> format=qcow2,cache=off,werror=enospc -drive \
> file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,bus=1,unit=0,format=raw -net \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.args
> index 877c0db..bf955a9 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.args
> @@ -1,5 +1,6 @@
> LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
> -pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
> +pc -m 214 -smp 1 -nographic \
> +-monitor control,unix:/tmp/test-monitor,server,nowait \
> -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,\
> format=qcow2,cache=off,werror=stop,rerror=stop -drive \
> file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,bus=1,unit=0,format=raw -net \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-wreport-rignore.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-wreport-rignore.args
> index 4879576..6bb9a93 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-wreport-rignore.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-wreport-rignore.args
> @@ -1,5 +1,6 @@
> LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
> -pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
> +pc -m 214 -smp 1 -nographic \
> +-monitor control,unix:/tmp/test-monitor,server,nowait \
> -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,\
> format=qcow2,cache=off,werror=report,rerror=ignore -drive \
> file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,bus=1,unit=0,format=raw -net \
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index a7ae925..4b72af7 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -171,7 +171,6 @@ struct testInfo {
> virBitmapPtr extraFlags;
> const char *migrateFrom;
> int migrateFd;
> - bool json;
> bool expectError;
> };
>
> @@ -191,7 +190,9 @@ testCompareXMLToArgvHelper(const void *data)
>
> result = testCompareXMLToArgvFiles(xml, args, info->extraFlags,
> info->migrateFrom, info->migrateFd,
> - info->json, info->expectError);
> + qemuCapsGet(info->extraFlags,
> + QEMU_CAPS_MONITOR_JSON),
> + info->expectError);
>
> cleanup:
> free(xml);
> @@ -206,7 +207,6 @@ mymain(void)
> {
> int ret = 0;
> char *map = NULL;
> - bool json = false;
>
> abs_top_srcdir = getenv("abs_top_srcdir");
> if (!abs_top_srcdir)
> @@ -234,7 +234,7 @@ mymain(void)
> # define DO_TEST_FULL(name, migrateFrom, migrateFd, expectError, ...) \
> do { \
> struct testInfo info = { \
> - name, NULL, migrateFrom, migrateFd, json, expectError \
> + name, NULL, migrateFrom, migrateFd, expectError \
> }; \
> if (!(info.extraFlags = qemuCapsNew())) \
> return EXIT_FAILURE; \
> @@ -585,13 +585,11 @@ mymain(void)
> QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
> QEMU_CAPS_PCI_MULTIFUNCTION);
>
> - json = true;
> DO_TEST("monitor-json", false, QEMU_CAPS_DEVICE,
> QEMU_CAPS_CHARDEV, QEMU_CAPS_MONITOR_JSON, QEMU_CAPS_NODEFCONFIG);
> DO_TEST("no-shutdown", false, QEMU_CAPS_DEVICE,
> QEMU_CAPS_CHARDEV, QEMU_CAPS_MONITOR_JSON, QEMU_CAPS_NODEFCONFIG,
> QEMU_CAPS_NO_SHUTDOWN);
> - json = false;
>
> free(driver.stateDir);
> virCapabilitiesFree(driver.caps);
Ack with the nits fixed.
Thanks
Wen Congyang
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list