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

[Michal Novotny]

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

Michal

-- 
Michal Novotny<minovotn at redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat