3:43 a.m.
On 07/06/2015 01:38 PM, Martin Kletzander wrote:
On Mon, Jul 06, 2015 at 10:23:59AM +0800, lhuang wrote:
>
> On 07/03/2015 08:56 PM, Martin Kletzander wrote:
>> On Wed, Jun 17, 2015 at 11:56:14AM +0800, Luyao Huang wrote:
>>> Rename qemuBuildShmemDevCmd to qemuBuildShmemDevStr and change the
>>> return type so that it can be reused in the device hotplug code later.
>>>
>>> And split the chardev creation part in a new function
>>> qemuBuildShmemBackendStr for reused in the device hotplug code later.
>>>
>>> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
>>> ---
>>> src/qemu/qemu_command.c | 70
>>> +++++++++++++++++++++++++++----------------------
>>> src/qemu/qemu_command.h | 7 +++++
>>> 2 files changed, 45 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index 636e040..0414f77 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -8433,9 +8433,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>>> return ret;
>>> }
>>>
>>> -static int
>>> -qemuBuildShmemDevCmd(virCommandPtr cmd,
>>> - virDomainDefPtr def,
>>> +char *
>>> +qemuBuildShmemDevStr(virDomainDefPtr def,
>>> virDomainShmemDefPtr shmem,
>>> virQEMUCapsPtr qemuCaps)
>>> {
>>> @@ -8489,14 +8488,38 @@ qemuBuildShmemDevCmd(virCommandPtr cmd,
>>> if (virBufferCheckError(&buf) < 0)
>>> goto error;
>>>
>>> - virCommandAddArg(cmd, "-device");
>>> - virCommandAddArgBuffer(cmd, &buf);
>>> -
>>> - return 0;
>>> + return virBufferContentAndReset(&buf);
>>>
>>
>> You should be able to just unconditionally do
>> return virBufferContentAndReset() here since it returns NULL if
>> there's a buf->error.
>>
>> ACK with that changed.
>>
>
> Right, i forgot that, thanks a lot for your review
>
Sorry, you cannot, there's a "goto error;" from some part of the code
where there is no buf->error set, so no change to this, you were
right.
No need to resend these first three, I'll push whatever is OK to go in
after I finish the review, and let you know what needs work.
Okay, got it, thanks a lot for your helps.
BR,
Luyao
Thanks,
Martin
> Luyao
>
>>> error:
>>> virBufferFreeAndReset(&buf);
>>> - return -1;
>>> + return NULL;
>>> +}
>>> +
>>> +char *
>>> +qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem,
>>> + virQEMUCapsPtr qemuCaps)
>>> +{
>>> + char *devstr = NULL;
>>> + virDomainChrSourceDef source = {
>>> + .type = VIR_DOMAIN_CHR_TYPE_UNIX,
>>> + .data.nix = {
>>> + .path = shmem->server.path,
>>> + .listen = false,
>>> + }
>>> + };
>>> +
>>> + if (!shmem->server.path &&
>>> + virAsprintf(&source.data.nix.path,
>>> + "/var/lib/libvirt/shmem-%s-sock",
>>> + shmem->name) < 0)
>>> + return NULL;
>>> +
>>> + devstr = qemuBuildChrChardevStr(&source, shmem->info.alias,
>>> qemuCaps);
>>> +
>>> + if (!shmem->server.path)
>>> + VIR_FREE(source.data.nix.path);
>>> +
>>> + return devstr;
>>> }
>>>
>>> static int
>>> @@ -8505,35 +8528,18 @@ qemuBuildShmemCommandLine(virCommandPtr cmd,
>>> virDomainShmemDefPtr shmem,
>>> virQEMUCapsPtr qemuCaps)
>>> {
>>> - if (qemuBuildShmemDevCmd(cmd, def, shmem, qemuCaps) < 0)
>>> + char *devstr = NULL;
>>> +
>>> + if (!(devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps)))
>>> return -1;
>>> + virCommandAddArgList(cmd, "-device", devstr, NULL);
>>> + VIR_FREE(devstr);
>>>
>>> if (shmem->server.enabled) {
>>> - char *devstr = NULL;
>>> - virDomainChrSourceDef source = {
>>> - .type = VIR_DOMAIN_CHR_TYPE_UNIX,
>>> - .data.nix = {
>>> - .path = shmem->server.path,
>>> - .listen = false,
>>> - }
>>> - };
>>> -
>>> - if (!shmem->server.path &&
>>> - virAsprintf(&source.data.nix.path,
>>> - "/var/lib/libvirt/shmem-%s-sock",
>>> - shmem->name) < 0)
>>> + if (!(devstr = qemuBuildShmemBackendStr(shmem, qemuCaps)))
>>> return -1;
>>>
>>> - devstr = qemuBuildChrChardevStr(&source,
>>> shmem->info.alias, qemuCaps);
>>> -
>>> - if (!shmem->server.path)
>>> - VIR_FREE(source.data.nix.path);
>>> -
>>> - if (!devstr)
>>> - return -1;
>>> -
>>> - virCommandAddArg(cmd, "-chardev");
>>> - virCommandAddArg(cmd, devstr);
>>> + virCommandAddArgList(cmd, "-chardev", devstr, NULL);
>>> VIR_FREE(devstr);
>>> }
>>>
>>> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
>>> index 0fc59a8..73f24dc 100644
>>> --- a/src/qemu/qemu_command.h
>>> +++ b/src/qemu/qemu_command.h
>>> @@ -194,6 +194,13 @@ int
>>> qemuBuildRNGBackendProps(virDomainRNGDefPtr rng,
>>> const char **type,
>>> virJSONValuePtr *props);
>>>
>>> +char *qemuBuildShmemDevStr(virDomainDefPtr def,
>>> + virDomainShmemDefPtr shmem,
>>> + virQEMUCapsPtr qemuCaps);
>>> +char *qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem,
>>> + virQEMUCapsPtr qemuCaps);
>>> +
>>> +
>>> int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
>>>
>>> /* Legacy, pre device support */
>>> --
>>> 1.8.3.1
>>>
>>> --
>>> libvir-list mailing list
>>> libvir-list(a)redhat.com
>>> https://www.redhat.com/mailman/listinfo/libvir-list
>