12:38 a.m.
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.
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