8:07 a.m.
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(a)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.
Michal
--
Michal Novotny<minovotn(a)redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat