Re: [libvirt] [PATCH 1/2] qemu_command: create prefixed alias to separate variable

2016-10-18 Thread Pavel Hrdina
On Tue, Oct 18, 2016 at 10:22:50AM -0400, John Ferlan wrote:
> 
> 
> On 10/18/2016 09:58 AM, Pavel Hrdina wrote:
> > Instead of typing the prefix every time we want to append parameters
> > to qemu command line use a variable that contains prefixed alias.
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  src/qemu/qemu_command.c | 35 ---
> >  1 file changed, 20 insertions(+), 15 deletions(-)
> > 
> 
> Why not create a qemu_alias.c helper that then can also be used in your
> followup patch?

That's a good point, I did not realize that we have qemu_alias.c.

I'll send v2, thanks.

Pavel


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

Re: [libvirt] [PATCH 1/2] qemu_command: create prefixed alias to separate variable

2016-10-18 Thread John Ferlan


On 10/18/2016 09:58 AM, Pavel Hrdina wrote:
> Instead of typing the prefix every time we want to append parameters
> to qemu command line use a variable that contains prefixed alias.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  src/qemu/qemu_command.c | 35 ---
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 

Why not create a qemu_alias.c helper that then can also be used in your
followup patch?

John

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 21fd85c..74f65c0 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4861,28 +4861,32 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>  {
>  virBuffer buf = VIR_BUFFER_INITIALIZER;
>  bool telnet;
> +char *charAlias = NULL;
> +
> +if (virAsprintf(&charAlias, "char%s", alias) < 0)
> +goto error;
>  
>  switch (dev->type) {
>  case VIR_DOMAIN_CHR_TYPE_NULL:
> -virBufferAsprintf(&buf, "null,id=char%s", alias);
> +virBufferAsprintf(&buf, "null,id=%s", charAlias);
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_VC:
> -virBufferAsprintf(&buf, "vc,id=char%s", alias);
> +virBufferAsprintf(&buf, "vc,id=%s", charAlias);
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_PTY:
> -virBufferAsprintf(&buf, "pty,id=char%s", alias);
> +virBufferAsprintf(&buf, "pty,id=%s", charAlias);
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_DEV:
> -virBufferAsprintf(&buf, "%s,id=char%s,path=%s",
> +virBufferAsprintf(&buf, "%s,id=%s,path=%s",
>STRPREFIX(alias, "parallel") ? "parport" : "tty",
> -  alias, dev->data.file.path);
> +  charAlias, dev->data.file.path);
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_FILE:
> -virBufferAsprintf(&buf, "file,id=char%s", alias);
> +virBufferAsprintf(&buf, "file,id=%s", charAlias);
>  
>  if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT &&
>  !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) {
> @@ -4898,12 +4902,12 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_PIPE:
> -virBufferAsprintf(&buf, "pipe,id=char%s,path=%s", alias,
> +virBufferAsprintf(&buf, "pipe,id=%s,path=%s", charAlias,
>dev->data.file.path);
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_STDIO:
> -virBufferAsprintf(&buf, "stdio,id=char%s", alias);
> +virBufferAsprintf(&buf, "stdio,id=%s", charAlias);
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_UDP: {
> @@ -4919,9 +4923,9 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>  bindService = "0";
>  
>  virBufferAsprintf(&buf,
> -  "udp,id=char%s,host=%s,port=%s,localaddr=%s,"
> +  "udp,id=%s,host=%s,port=%s,localaddr=%s,"
>"localport=%s",
> -  alias,
> +  charAlias,
>connectHost,
>dev->data.udp.connectService,
>bindHost, bindService);
> @@ -4930,8 +4934,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>  case VIR_DOMAIN_CHR_TYPE_TCP:
>  telnet = dev->data.tcp.protocol == 
> VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
>  virBufferAsprintf(&buf,
> -  "socket,id=char%s,host=%s,port=%s%s",
> -  alias,
> +  "socket,id=%s,host=%s,port=%s%s",
> +  charAlias,
>dev->data.tcp.host,
>dev->data.tcp.service,
>telnet ? ",telnet" : "");
> @@ -4956,7 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_UNIX:
> -virBufferAsprintf(&buf, "socket,id=char%s,path=", alias);
> +virBufferAsprintf(&buf, "socket,id=%s,path=", charAlias);
>  virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
>  if (dev->data.nix.listen)
>  virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1);
> @@ -4968,7 +4972,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> _("spicevmc not supported in this QEMU binary"));
>  goto error;
>  }
> -virBufferAsprintf(&buf, "spicevmc,id=char%s,name=%s", alias,
> +virBufferAsprintf(&buf, "spicevmc,id=%s,name=%s", charAlias,
>
> virDomainChrSpicevmcTypeToString(dev->data.spicevmc));
>  break;
>  
> @@ -4978,7 +4982,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> _("spiceport not supported in this QEMU binary"));
>  goto error;
>  }
> -virBufferAsprintf(&buf, "spi

[libvirt] [PATCH 1/2] qemu_command: create prefixed alias to separate variable

2016-10-18 Thread Pavel Hrdina
Instead of typing the prefix every time we want to append parameters
to qemu command line use a variable that contains prefixed alias.

Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_command.c | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 21fd85c..74f65c0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4861,28 +4861,32 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 bool telnet;
+char *charAlias = NULL;
+
+if (virAsprintf(&charAlias, "char%s", alias) < 0)
+goto error;
 
 switch (dev->type) {
 case VIR_DOMAIN_CHR_TYPE_NULL:
-virBufferAsprintf(&buf, "null,id=char%s", alias);
+virBufferAsprintf(&buf, "null,id=%s", charAlias);
 break;
 
 case VIR_DOMAIN_CHR_TYPE_VC:
-virBufferAsprintf(&buf, "vc,id=char%s", alias);
+virBufferAsprintf(&buf, "vc,id=%s", charAlias);
 break;
 
 case VIR_DOMAIN_CHR_TYPE_PTY:
-virBufferAsprintf(&buf, "pty,id=char%s", alias);
+virBufferAsprintf(&buf, "pty,id=%s", charAlias);
 break;
 
 case VIR_DOMAIN_CHR_TYPE_DEV:
-virBufferAsprintf(&buf, "%s,id=char%s,path=%s",
+virBufferAsprintf(&buf, "%s,id=%s,path=%s",
   STRPREFIX(alias, "parallel") ? "parport" : "tty",
-  alias, dev->data.file.path);
+  charAlias, dev->data.file.path);
 break;
 
 case VIR_DOMAIN_CHR_TYPE_FILE:
-virBufferAsprintf(&buf, "file,id=char%s", alias);
+virBufferAsprintf(&buf, "file,id=%s", charAlias);
 
 if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT &&
 !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) {
@@ -4898,12 +4902,12 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 break;
 
 case VIR_DOMAIN_CHR_TYPE_PIPE:
-virBufferAsprintf(&buf, "pipe,id=char%s,path=%s", alias,
+virBufferAsprintf(&buf, "pipe,id=%s,path=%s", charAlias,
   dev->data.file.path);
 break;
 
 case VIR_DOMAIN_CHR_TYPE_STDIO:
-virBufferAsprintf(&buf, "stdio,id=char%s", alias);
+virBufferAsprintf(&buf, "stdio,id=%s", charAlias);
 break;
 
 case VIR_DOMAIN_CHR_TYPE_UDP: {
@@ -4919,9 +4923,9 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 bindService = "0";
 
 virBufferAsprintf(&buf,
-  "udp,id=char%s,host=%s,port=%s,localaddr=%s,"
+  "udp,id=%s,host=%s,port=%s,localaddr=%s,"
   "localport=%s",
-  alias,
+  charAlias,
   connectHost,
   dev->data.udp.connectService,
   bindHost, bindService);
@@ -4930,8 +4934,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 case VIR_DOMAIN_CHR_TYPE_TCP:
 telnet = dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
 virBufferAsprintf(&buf,
-  "socket,id=char%s,host=%s,port=%s%s",
-  alias,
+  "socket,id=%s,host=%s,port=%s%s",
+  charAlias,
   dev->data.tcp.host,
   dev->data.tcp.service,
   telnet ? ",telnet" : "");
@@ -4956,7 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 break;
 
 case VIR_DOMAIN_CHR_TYPE_UNIX:
-virBufferAsprintf(&buf, "socket,id=char%s,path=", alias);
+virBufferAsprintf(&buf, "socket,id=%s,path=", charAlias);
 virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
 if (dev->data.nix.listen)
 virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1);
@@ -4968,7 +4972,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
_("spicevmc not supported in this QEMU binary"));
 goto error;
 }
-virBufferAsprintf(&buf, "spicevmc,id=char%s,name=%s", alias,
+virBufferAsprintf(&buf, "spicevmc,id=%s,name=%s", charAlias,
   
virDomainChrSpicevmcTypeToString(dev->data.spicevmc));
 break;
 
@@ -4978,7 +4982,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
_("spiceport not supported in this QEMU binary"));
 goto error;
 }
-virBufferAsprintf(&buf, "spiceport,id=char%s,name=%s", alias,
+virBufferAsprintf(&buf, "spiceport,id=%s,name=%s", charAlias,
   dev->data.spiceport.channel);
 break;
 
@@ -5007,6 +5011,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 return virBufferContentAndReset(&buf);
 
  error:
+VIR_FREE(charAlias);
 virBufferFreeAndReset(&buf);