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

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

2015-07-06 Thread lhuang



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 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



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




--
libvir-list mailing list

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

2015-07-05 Thread Martin Kletzander


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 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



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




signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com

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

2015-07-05 Thread 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

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

2015-07-03 Thread Martin Kletzander


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.


 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


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information