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

Re: [libvirt] [PATCH] Add support for multiple serial ports to Xen driver



On Thu, Jan 20, 2011 at 03:07:55PM +0100, Michal Novotny wrote:
> On 01/20/2011 03:07 PM, Daniel P. Berrange wrote:
> >On Thu, Jan 20, 2011 at 03:06:11PM +0100, Michal Novotny wrote:
> >>On 01/20/2011 02:53 PM, Daniel P. Berrange wrote:
> >>>On Thu, Jan 20, 2011 at 01:20:50PM +0100, Michal Novotny wrote:
> >>>>Hi,
> >>>>this is the patch for to support multiple serial ports
> >>>>for Xen driver. The definition for Xen has been
> >>>>introduced in BZ #614004 and this is adding
> >>>>support to libvirt-based tools.
> >>>>
> >>>>The patch was originally written for RHEL-5 and libvirt
> >>>>0.8.2 (RHEL-5.6) where it has been tested using
> >>>>the virsh command and correct device creation has
> >>>>been confirmed in the xend.log to have the same data
> >>>>for serial ports using both `xm create` and `virsh
> >>>>start` commands. Also, domains with both single and
> >>>>multiple serial ports has been tested to confirm
> >>>>it won't introduce any regression and everything
> >>>>was working fine according to my testing. This patch
> >>>>is the forward-port of RHEL-5 version of the patch.
> >>>>
> >>>>Michal
> >>>>
> >>>>Signed-off-by: Michal Novotny<minovotn redhat com>
> >>>>---
> >>>>  src/xen/xend_internal.c |   19 ++++++++++++-
> >>>>  src/xen/xm_internal.c   |   66 +++++++++++++++++++++++++++++++++++++++--------
> >>>>  3 files changed, 73 insertions(+), 14 deletions(-)
> >>>>
> >>>>diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> >>>>index 44d5a22..35cdd3c 100644
> >>>>--- a/src/xen/xend_internal.c
> >>>>+++ b/src/xen/xend_internal.c
> >>>>@@ -5959,8 +5959,23 @@ xenDaemonFormatSxpr(virConnectPtr conn,
> >>>>              }
> >>>>              if (def->serials) {
> >>>>                  virBufferAddLit(&buf, "(serial ");
> >>>>-                if (xenDaemonFormatSxprChr(def->serials[0],&buf)<   0)
> >>>>-                    goto error;
> >>>>+                /*
> >>>>+                 * If domain doesn't have multiple serial ports defined we
> >>>>+                 * keep the old-style formatting not to fail the sexpr tests
> >>>>+                 */
> >>>>+                if (def->nserials>   1) {
> >>>>+                    for (i = 0; i<   def->nserials; i++) {
> >>>>+                        virBufferAddLit(&buf, "(");
> >>>>+                        if (xenDaemonFormatSxprChr(def->serials[i],&buf)<   0)
> >>>>+                            goto error;
> >>>>+                        virBufferAddLit(&buf, ")");
> >>>>+                    }
> >>>>+                }
> >>>>+                else {
> >>>>+                    if (xenDaemonFormatSxprChr(def->serials[0],&buf)<   0)
> >>>>+                        goto error;
> >>>>+                }
> >>>>+
> >>>>                  virBufferAddLit(&buf, ")");
> >>>>              } else {
> >>>>                  virBufferAddLit(&buf, "(serial none)");
> >>>>diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> >>>>index bfb6698..1bb62d7 100644
> >>>>--- a/src/xen/xm_internal.c
> >>>>+++ b/src/xen/xm_internal.c
> >>>>@@ -1432,20 +1432,64 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
> >>>>              chr = NULL;
> >>>>          }
> >>>>
> >>>>-        if (xenXMConfigGetString(conf, "serial",&str, NULL)<   0)
> >>>>-            goto cleanup;
> >>>>-        if (str&&   STRNEQ(str, "none")&&
> >>>>-            !(chr = xenDaemonParseSxprChar(str, NULL)))
> >>>>-            goto cleanup;
> >>>>+        /* Try to get the list of values to support multiple serial ports */
> >>>>+        list = virConfGetValue(conf, "serial");
> >>>>+        if (list&&   list->type == VIR_CONF_LIST) {
> >>>>+            list = list->list;
> >>>>+            while (list) {
> >>>>+                char *port;
> >>>>+
> >>>>+                if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
> >>>>+                    goto skipport;
> >>>>+
> >>>>+                port = list->str;
> >>>>+                if (VIR_ALLOC(chr)<   0)
> >>>>+                    goto no_memory;
> >>>>
> >>>>-        if (chr) {
> >>>>-            if (VIR_ALLOC_N(def->serials, 1)<   0) {
> >>>>+                if (!(chr = xenDaemonParseSxprChar(port, NULL)))
> >>>>+                    goto skipport;
> >>>>+
> >>>>+                if (VIR_REALLOC_N(def->serials, def->nserials+1)<   0)
> >>>>+                    goto no_memory;
> >>>>+
> >>>>+                chr->targetType = VIR_DOMAIN_CHR_TARGET_TYPE_SERIAL;
> >>>>+                chr->type = VIR_DOMAIN_CHR_TYPE_PTY;
> >>>>+
> >>>>+                /* Implement device type of serial port when appropriate */
> >>>>+                if (STRPREFIX(port, "/dev")) {
> >>>>+                    chr->type = VIR_DOMAIN_CHR_TYPE_DEV;
> >>>>+                    chr->target.port = def->nserials;
> >>>>+                    chr->data.file.path = strdup(port);
> >>>>+
> >>>>+                    if (!chr->data.file.path)
> >>>>+                        goto no_memory;
> >>>>+                }
> >>>>+
> >>>>+                def->serials[def->nserials++] = chr;
> >>>>+                chr = NULL;
> >>>>+
> >>>>+                skipport:
> >>>>+                list = list->next;
> >>>>                  virDomainChrDefFree(chr);
> >>>>-                goto no_memory;
> >>>>              }
> >>>>-            chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL;
> >>>>-            def->serials[0] = chr;
> >>>>-            def->nserials++;
> >>>>+        }
> >>>>+        /* If domain is not using multiple serial ports we parse data old way */
> >>>>+        else {
> >>>>+            if (xenXMConfigGetString(conf, "serial",&str, NULL)<   0)
> >>>>+                goto cleanup;
> >>>>+            if (str&&   STRNEQ(str, "none")&&
> >>>>+                !(chr = xenDaemonParseSxprChar(str, NULL)))
> >>>>+                goto cleanup;
> >>>>+
> >>>>+            if (chr) {
> >>>>+                if (VIR_ALLOC_N(def->serials, 1)<   0) {
> >>>>+                    virDomainChrDefFree(chr);
> >>>>+                    goto no_memory;
> >>>>+                }
> >>>>+                chr->targetType = VIR_DOMAIN_CHR_TARGET_TYPE_SERIAL;
> >>>>+                def->serials[0] = chr;
> >>>>+                def->nserials++;
> >>>>+            }
> >>>>          }
> >>>>      } else {
> >>>>          if (!(def->console = xenDaemonParseSxprChar("pty", NULL)))
> >>>Hmm, unless I'm missing something, this patch only seems
> >>>todo half the job. It lets you parse XM configs, or generate
> >>>SEXPRS, needed to start/create guests. It doesn't let you
> >>>parse SEXPRS to query XML, or write XM configs to save an
> >>>updated guest config.
> >>>
> >>>Daniel
> >>Good point Daniel. What's the good test for that? Just to issue
> >>virsh edit and try to change&  save something ?
> >The tests/ directory has   xmconfigtest, xensexpr2xmltest and
> >xenxml2sexprtest which can be used to validate all directions
> >by giving sample config files
> >
> >Daniel
> Well, those tests (all except xencapstest) passed the testing.

That would be because you didn't add any more config files
to actually test the multiple serial port style configs....

Daniel


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