[libvirt] [PATCH 03/10] qemu: Refactor creation of shared memory device commandline

Martin Kletzander mkletzan at redhat.com
Mon Jul 6 05:38:51 UTC 2015


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 at 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 at redhat.com
>>>https://www.redhat.com/mailman/listinfo/libvir-list
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150706/6e5a324d/attachment-0001.sig>


More information about the libvir-list mailing list