[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2] qemu: simplify use of HAVE_YAJL



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]