[libvirt] [PATCH 5/8] Change extract pidfile & monitor config from QEMU command line

[Matthias Bolte]

2011/7/4 Daniel P. Berrange <berrange at redhat.com>:
> When converting QEMU argv into a virDomainDefPtr, also extract
> the pidfile, monitor character device config and the monitor
> mode.
>
> * src/qemu/qemu_command.c, src/qemu/qemu_command.h: Extract
>  pidfile & monitor config from QEMU argv
> * src/qemu/qemu_driver.c, tests/qemuargv2xmltest.c: Add extra
>  params when calling qemuParseCommandLineString
> ---
>  src/qemu/qemu_command.c  |  150 +++++++++++++++++++++++++++------------------
>  src/qemu/qemu_command.h  |   10 +++-
>  src/qemu/qemu_driver.c   |    9 +++-
>  tests/qemuargv2xmltest.c |    3 +-
>  4 files changed, 108 insertions(+), 64 deletions(-)

> @@ -6523,11 +6548,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
>
>     VIR_FREE(nics);
>
> -    if (!def->name) {
> -        if (!(def->name = strdup("unnamed")))
> -            goto no_memory;
> -    }
> -
>     if (virDomainDefAddImplicitControllers(def) < 0)
>         goto error;

Okay you moved this to qemuDomainXMLFromNative, so now a direct call
to qemuParseCommandLine will produce an invalid virDomainDefPtr as
def->name can be NULL when -name was not given in the command line. It
seems that this is okay for now as the only caller is currently
qemuDomainXMLFromNative. You added a second caller in 8/8 that
explicitly deals with def->name == NULL.

Might be worth a comment to qemuParseCommandLine that mentions this.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 363a361..9486594 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3887,11 +3887,18 @@ static char *qemuDomainXMLFromNative(virConnectPtr conn,
>     }
>
>     qemuDriverLock(driver);
> -    def = qemuParseCommandLineString(driver->caps, config);
> +    def = qemuParseCommandLineString(driver->caps, config,
> +                                     NULL, NULL, NULL);
>     qemuDriverUnlock(driver);
>     if (!def)
>         goto cleanup;
>
> +    if (!def->name &&
> +        !(def->name = strdup("unnamed"))) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
>     xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE);
>

ACK.

-- 
Matthias Bolte
http://photron.blogspot.com