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@xxxxxxxxxx>
> >>---
> >> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list