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

[lhuang]

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 <lhu...@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

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list