[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 02/10] Convert character devices over to use -device



On Tue, Dec 15, 2009 at 03:14:42PM +0000, Daniel P. Berrange wrote:
> The current character device syntax uses either
> 
>   -serial tty,path=/dev/ttyS2
> 
> Or
> 
>   -chardev tty,id=serial0,path=/dev/ttyS2 -serial chardev:serial0
> 
> With the new -device support, we now prefer
> 
>   -chardev file,id=serial0,path=/tmp/serial.log -device isa-serial,chardev=serial0
> 
> This patch changes the existing -chardev syntax to use this new
> scheme, and fallbacks to the old plain -serial syntax for old
> QEMU.
> 
> The monitor device changes to
> 
>   -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor
> 
> In addition, this patch adds --nodefaults, which kills off the
> default serial, parallel, vga and nic devices. THis avoids the
> need for us to explicitly turn each off
> ---
>  src/qemu/qemu_conf.c                               |   74 +++++++++++++-------
>  src/qemu/qemu_conf.h                               |    1 +
>  .../qemuxml2argv-channel-guestfwd.args             |    2 +-
>  .../qemuxml2argv-console-compat-chardev.args       |    2 +-
>  .../qemuxml2argv-parallel-tcp-chardev.args         |    2 +-
>  .../qemuxml2argv-serial-dev-chardev.args           |    2 +-
>  .../qemuxml2argv-serial-file-chardev.args          |    2 +-
>  .../qemuxml2argv-serial-many-chardev.args          |    2 +-
>  .../qemuxml2argv-serial-pty-chardev.args           |    2 +-
>  .../qemuxml2argv-serial-tcp-chardev.args           |    2 +-
>  .../qemuxml2argv-serial-tcp-telnet-chardev.args    |    2 +-
>  .../qemuxml2argv-serial-udp-chardev.args           |    2 +-
>  .../qemuxml2argv-serial-unix-chardev.args          |    2 +-
>  .../qemuxml2argv-serial-vc-chardev.args            |    2 +-
>  tests/qemuxml2argvtest.c                           |   26 ++++----
>  15 files changed, 74 insertions(+), 51 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 61e719d..16e8d2c 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -909,6 +909,8 @@ static unsigned int qemudComputeCmdFlags(const char *help,
>          flags |= QEMUD_CMD_FLAG_MEM_PATH;
>      if (strstr(help, "-chardev"))
>          flags |= QEMUD_CMD_FLAG_CHARDEV;
> +    if (strstr(help, "-device"))
> +        flags |= QEMUD_CMD_FLAG_DEVICE;

  hum ... shouldn't we look for a space of some sort after the option
strings, to make sure we match the right string. Not specific to this
but might be safer.

>      if (version >= 9000)
>          flags |= QEMUD_CMD_FLAG_VNC_COLON;
> @@ -1945,6 +1947,9 @@ int qemudBuildCommandLine(virConnectPtr conn,
>      if (!def->graphics)
>          ADD_ARG_LIT("-nographic");
>  
> +    if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)
> +        ADD_ARG_LIT("-nodefaults");
> +
>      if (monitor_chr) {
>          virBuffer buf = VIR_BUFFER_INITIALIZER;
>  
> @@ -1959,26 +1964,32 @@ int qemudBuildCommandLine(virConnectPtr conn,
>              ADD_ARG_LIT("-chardev");
>              ADD_ARG(virBufferContentAndReset(&buf));
>  
> -            if (monitor_json)
> -                virBufferAddLit(&buf, "control,");
> +            virBufferAddLit(&buf, "chardev=monitor");
>  
> -            virBufferAddLit(&buf, "chardev:monitor");
> -        }
> +            if (virBufferError(&buf)) {
> +                virBufferFreeAndReset(&buf);
> +                goto no_memory;
> +            }
>  
> -        else {
> +            ADD_ARG_LIT("-mon");
> +            if (monitor_json)
> +                ADD_ARG_LIT("chardev=monitor,mode=control");
> +            else
> +                ADD_ARG_LIT("chardev=monitor,mode=readline");
> +        } else {
>              if (monitor_json)
>                  virBufferAddLit(&buf, "control,");
>  
>              qemudBuildCommandLineChrDevStr(monitor_chr, &buf);
> -        }
>  
> -        if (virBufferError(&buf)) {
> -            virBufferFreeAndReset(&buf);
> -            goto no_memory;
> -        }
> +            if (virBufferError(&buf)) {
> +                virBufferFreeAndReset(&buf);
> +                goto no_memory;
> +            }
>  
> -        ADD_ARG_LIT("-monitor");
> -        ADD_ARG_LIT(virBufferContentAndReset(&buf));
> +            ADD_ARG_LIT("-monitor");
> +            ADD_ARG(virBufferContentAndReset(&buf));
> +        }
>      }
>  
>      if (def->localtime)
> @@ -2172,8 +2183,11 @@ int qemudBuildCommandLine(virConnectPtr conn,
>      }
>  
>      if (!def->nnets) {
> -        ADD_ARG_LIT("-net");
> -        ADD_ARG_LIT("none");
> +        /* If we have -device, then we set -nodefault alrady */
> +        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
> +            ADD_ARG_LIT("-net");
> +            ADD_ARG_LIT("none");
> +        }
>      } else {
>          for (i = 0 ; i < def->nnets ; i++) {
>              virDomainNetDefPtr net = def->nets[i];
> @@ -2232,15 +2246,19 @@ int qemudBuildCommandLine(virConnectPtr conn,
>      }
>  
>      if (!def->nserials) {
> -        ADD_ARG_LIT("-serial");
> -        ADD_ARG_LIT("none");
> +        /* If we have -device, then we set -nodefault alrady */
> +        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
> +            ADD_ARG_LIT("-serial");
> +            ADD_ARG_LIT("none");
> +        }
>      } else {
>          for (i = 0 ; i < def->nserials ; i++) {
>              virBuffer buf = VIR_BUFFER_INITIALIZER;
>              virDomainChrDefPtr serial = def->serials[i];
>  
> -            /* Use -chardev if it's available */
> -            if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) {
> +            /* Use -chardev with -device if they are available */
> +            if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) &&
> +                (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
>                  char id[16];
>  
>                  if (snprintf(id, sizeof(id), "serial%i", i) > sizeof(id))
> @@ -2255,13 +2273,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
>                  ADD_ARG_LIT("-chardev");
>                  ADD_ARG(virBufferContentAndReset(&buf));
>  
> -                virBufferVSprintf(&buf, "chardev:%s", id);
> +                virBufferVSprintf(&buf, "isa-serial,chardev=%s", id);
>                  if (virBufferError(&buf)) {
>                      virBufferFreeAndReset(&buf);
>                      goto no_memory;
>                  }
>  
> -                ADD_ARG_LIT("-serial");
> +                ADD_ARG_LIT("-device");
>                  ADD_ARG(virBufferContentAndReset(&buf));
>              }
>  
> @@ -2279,15 +2297,19 @@ int qemudBuildCommandLine(virConnectPtr conn,
>      }
>  
>      if (!def->nparallels) {
> -        ADD_ARG_LIT("-parallel");
> -        ADD_ARG_LIT("none");
> +        /* If we have -device, then we set -nodefault alrady */
> +        if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
> +            ADD_ARG_LIT("-parallel");
> +            ADD_ARG_LIT("none");
> +        }
>      } else {
>          for (i = 0 ; i < def->nparallels ; i++) {
>              virBuffer buf = VIR_BUFFER_INITIALIZER;
>              virDomainChrDefPtr parallel = def->parallels[i];
>  
> -            /* Use -chardev if it's available */
> -            if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) {
> +            /* Use -chardev with -device if they are available */
> +            if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) &&
> +                (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
>                  char id[16];
>  
>                  if (snprintf(id, sizeof(id), "parallel%i", i) > sizeof(id))
> @@ -2302,13 +2324,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
>                  ADD_ARG_LIT("-chardev");
>                  ADD_ARG(virBufferContentAndReset(&buf));
>  
> -                virBufferVSprintf(&buf, "chardev:%s", id);
> +                virBufferVSprintf(&buf, "isa-parallel,chardev=%s", id);
>                  if (virBufferError(&buf)) {
>                      virBufferFreeAndReset(&buf);
>                      goto no_memory;
>                  }
>  
> -                ADD_ARG_LIT("-parallel");
> +                ADD_ARG_LIT("-device");
>                  ADD_ARG(virBufferContentAndReset(&buf));
>              }
>  
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index cd59d4c..840b749 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -76,6 +76,7 @@ enum qemud_cmd_flags {
>      QEMUD_CMD_FLAG_ENABLE_KVM    = (1 << 23), /* Is the -enable-kvm flag available to "enable KVM full virtualization support" */
>      QEMUD_CMD_FLAG_0_12          = (1 << 24),
>      QEMUD_CMD_FLAG_MONITOR_JSON  = QEMUD_CMD_FLAG_0_12, /* JSON mode for monitor */
> +    QEMUD_CMD_FLAG_DEVICE        = (1 << 25), /* Is the new -chardev arg available */
>  };
>  
>  /* Main driver state */

  I'm starting to worry about exhausting the enum size ... we are
  getting close, and something allowing for the continuous expansion of
  QEmu options will be needed soon !

ACK

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]