[Libvir] [PATCH] sound support for qemu and xen
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form: <sound driver='drivername'/> Where driver name can be pcspk, sb16, es1370, or all. Everything seems to be in working order but I have a few implementation questions: 1) Should multiple drivers be able to be specified? qemu accommodates this, allowing '-soundhw sb16,pcspk' for example. If this should be allowed, what should the xml format be? 2) Should acceptable driver options be hardcoded? The other option is to just pass the input straight to qemu. This patch has the options hardcoded. Also I realize this will probably need to be rediff'd around Dan's serial + parallel device patch, but I figured I would just get this out there. Thanks, Cole diff --git a/src/qemu_conf.c b/src/qemu_conf.c index d9b82b2..bfd9ba4 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1011,6 +1011,41 @@ static int qemudParseInputXML(virConnectPtr conn, return -1; } +/* Sound device helper functions */ +static int qemudSoundDriverFromString(virConnectPtr conn, + const char *driver) { + if (STREQ(driver, "all")) { + return QEMU_SOUND_ALL; + } else if (STREQ(driver, "sb16")) { + return QEMU_SOUND_SB16; + } else if (STREQ(driver, "es1370")) { + return QEMU_SOUND_ES1370; + } else if (STREQ(driver, "pcspk")) { + return QEMU_SOUND_PCSPK; + } + + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, + _("invalid sound driver '%s'"), driver); + return -1; +} + +static const char *qemudSoundDriverToString(virConnectPtr conn, + const int driver) { + + if (driver == QEMU_SOUND_ALL) { + return "all"; + } else if (driver == QEMU_SOUND_SB16) { + return "sb16"; + } else if (driver == QEMU_SOUND_ES1370) { + return "es1370"; + } else if (driver == QEMU_SOUND_PCSPK) { + return "pcspk"; + } + + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("invalid sound driver '%d'"), driver); + return NULL; +} /* * Parses a libvirt XML definition of a guest, and populates the @@ -1486,6 +1521,25 @@ static struct qemud_vm_def *qemudParseXML(virConnectPtr conn, } } xmlXPathFreeObject(obj); + + /* Parse sound driver xml */ + obj = xmlXPathEval(BAD_CAST "/domain/devices/sound", ctxt); + if ((obj == NULL) || (obj->type != XPATH_NODESET) || + (obj->nodesetval == NULL) || (obj->nodesetval->nodeNr == 0)) { + def->soundDriver = QEMU_SOUND_NONE; + } else if ((prop = xmlGetProp(obj->nodesetval->nodeTab[0], + BAD_CAST "driver"))) { + + if ((def->soundDriver = qemudSoundDriverFromString(conn, + (char *) prop)) < 0) + goto error; + + xmlFree(prop); + prop = NULL; + } else { + def->soundDriver = QEMU_SOUND_NONE; + } + xmlXPathFreeObject(obj); obj = NULL; /* If graphics are enabled, there's an implicit PS2 mouse */ @@ -1694,6 +1748,7 @@ int qemudBuildCommandLine(virConnectPtr conn, (vm->def->os.cmdline[0] ? 2 : 0) + /* cmdline */ (vm->def->graphicsType == QEMUD_GRAPHICS_VNC ? 2 : (vm->def->graphicsType == QEMUD_GRAPHICS_SDL ? 0 : 1)) + /* graphics */ + (vm->def->soundDriver == QEMU_SOUND_NONE ? 0 : 2) + /* sound */ (vm->migrateFrom[0] ? 3 : 0); /* migrateFrom */ snprintf(memory, sizeof(memory), "%lu", vm->def->memory/1024); @@ -1970,6 +2025,14 @@ int qemudBuildCommandLine(virConnectPtr conn, /* SDL is the default. no args needed */ } + /* Add sound hardware */ + if (vm->def->soundDriver != QEMU_SOUND_NONE) { + if (!((*argv)[++n] = strdup("-soundhw"))) + goto no_memory; + if (!((*argv)[++n] = strdup((char *) qemudSoundDriverToString(conn, vm->def->soundDriver)))) + goto no_memory; + } + if (vm->migrateFrom[0]) { if (!((*argv)[++n] = strdup("-S"))) goto no_memory; @@ -3125,7 +3188,11 @@ char *qemudGenerateXML(virConnectPtr conn, break; } - if (def->graphicsType == QEMUD_GRAPHICS_VNC) { + if (def->soundDriver != QEMU_SOUND_NONE) { + if (virBufferVSprintf(buf, " <sound driver='%s'/>\n", + qemudSoundDriverToString(conn, + def->soundDriver)) < 0) + goto no_memory; } if (virBufferAddLit(buf, " </devices>\n") < 0) diff --git a/src/qemu_conf.h b/src/qemu_conf.h index c59b1fa..1383c10 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -136,6 +136,14 @@ struct qemud_vm_input_def { struct qemud_vm_input_def *next; }; +enum qemu_vm_sound_driver { + QEMU_SOUND_NONE, + QEMU_SOUND_ALL, + QEMU_SOUND_SB16, + QEMU_SOUND_ES1370, + QEMU_SOUND_PCSPK, +}; + /* Flags for the 'type' field in next struct */ enum qemud_vm_device_type { QEMUD_DEVICE_DISK, @@ -214,6 +222,7 @@ struct qemud_vm_def { int vncActivePort; char vncListen[BR_INET_ADDR_MAXLEN]; char *keymap; + int soundDriver; int ndisks; struct qemud_vm_disk_def *disks; diff --git a/src/xend_internal.c b/src/xend_internal.c index 6ba4571..b470731 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -1783,6 +1783,18 @@ xend_parse_sexp_desc(virConnectPtr conn, struct sexpr *root, } } } + + if (sexpr_node(root, "domain/image/hvm/soundhw")) { + tmp = sexpr_node(root, "domain/image/hvm/soundhw"); + if (tmp && *tmp) { + if (STREQ(tmp, "all") || + STREQ(tmp, "pcspk") || + STREQ(tmp, "sb16") || + STREQ(tmp, "es1370")) { + virBufferVSprintf(&buf, " <sound driver='%s'/>\n", tmp); + } + } + } } /* Graphics device (HVM <= 3.0.4, or PV <= 3.0.3) vnc config */ diff --git a/src/xm_internal.c b/src/xm_internal.c index 3d845dc..91c7cf1 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -934,6 +934,15 @@ char *xenXMDomainFormatXML(virConnectPtr conn, virConfPtr conf) { /* Ignore else branch - probably some other non-input device we don't support in libvirt yet */ } + + if ((xenXMConfigGetString(conf, "soundhw", &str) == 0) && str) { + if (STREQ(str, "all") || + STREQ(str, "pcspk") || + STREQ(str, "sb16") || + STREQ(str, "es1370")) { + virBufferVSprintf(buf, " <sound driver='%s'/>\n", str); + } + } } /* HVM guests, or old PV guests use this config format */ @@ -2081,6 +2090,11 @@ virConfPtr xenXMParseXMLToConfig(virConnectPtr conn, const char *xml) { if (xenXMConfigSetStringFromXPath(conn, conf, ctxt, "usbdevice", "string(/domain/devices/input[@bus='usb' or (not(@bus) and @type='tablet')]/@type)", 1, "cannot set the usbdevice parameter") < 0) goto error; + + if (xenXMConfigSetStringFromXPath(conn, conf, ctxt, "soundhw", + "string(/domain/devices/sound[@driver]", + 1, "cannot set soundhw parameter") < 0) + goto error; } if (hvm || priv->xendConfigVersion < 3) { diff --git a/src/xml.c b/src/xml.c index 8e95103..f4bfb29 100644 --- a/src/xml.c +++ b/src/xml.c @@ -877,6 +877,14 @@ virDomainParseXMLOSDescHVM(virConnectPtr conn, xmlNodePtr node, nodes = NULL; } + cur = virXPathNode("/domain/devices/sound", ctxt); + if (cur) { + xmlChar *driver = NULL; + driver = xmlGetProp(cur, (xmlChar *) "driver"); + if (driver) + virBufferVSprintf(buf, "(soundhw '%s')", driver); + xmlFree(driver); + } res = virXPathBoolean("count(domain/devices/console) > 0", ctxt); if (res < 0) {
On Mon, Apr 21, 2008 at 01:02:35PM -0400, Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form:
<sound driver='drivername'/>
Where driver name can be pcspk, sb16, es1370, or all.
I'd like to use 'model' instead of 'driver', since we use 'model' in the network driver to specify the type of hardware and will do the same with disks too in the future. ie <sound model='sb16'/>
Everything seems to be in working order but I have a few implementation questions:
1) Should multiple drivers be able to be specified? qemu accommodates this, allowing '-soundhw sb16,pcspk' for example. If this should be allowed, what should the xml format be?
It should be represented as multiple <source> tags in the XML. The fact that is multiplexes onto a single -soundhw arg is an implementation detail that should remain hidden.
2) Should acceptable driver options be hardcoded? The other option is to just pass the input straight to qemu. This patch has the options hardcoded.
I prefer to have it hardcoded because it lets us explicitly block the string 'all'. The string 'all' is an implementation detail to which no reliable semantics can be assigned and should not be allowed by libvirt. Regards, Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
On Mon, Apr 21, 2008 at 01:02:35PM -0400, Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form:
<sound driver='drivername'/>
Where driver name can be pcspk, sb16, es1370, or all.
I'd like to use 'model' instead of 'driver', since we use 'model' in the network driver to specify the type of hardware and will do the same with disks too in the future.
ie
<sound model='sb16'/>
Makes sense. I'll make the change.
Everything seems to be in working order but I have a few implementation questions:
1) Should multiple drivers be able to be specified? qemu accommodates this, allowing '-soundhw sb16,pcspk' for example. If this should be allowed, what should the xml format be?
It should be represented as multiple <source> tags in the XML. The fact that is multiplexes onto a single -soundhw arg is an implementation detail that should remain hidden.
Did you mean <sound> tags? Something like: <sound model='d1'/> <sound model='d2'/>
2) Should acceptable driver options be hardcoded? The other option is to just pass the input straight to qemu. This patch has the options hardcoded.
I prefer to have it hardcoded because it lets us explicitly block the string 'all'. The string 'all' is an implementation detail to which no reliable semantics can be assigned and should not be allowed by libvirt.
Okay, I follow. I'll remove 'all' from the whitelist. Thanks, Cole
On Mon, Apr 21, 2008 at 02:20:29PM -0400, Cole Robinson wrote:
Daniel P. Berrange wrote:
On Mon, Apr 21, 2008 at 01:02:35PM -0400, Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form:
<sound driver='drivername'/>
Where driver name can be pcspk, sb16, es1370, or all.
I'd like to use 'model' instead of 'driver', since we use 'model' in the network driver to specify the type of hardware and will do the same with disks too in the future.
ie
<sound model='sb16'/>
Makes sense. I'll make the change.
Everything seems to be in working order but I have a few implementation questions:
1) Should multiple drivers be able to be specified? qemu accommodates this, allowing '-soundhw sb16,pcspk' for example. If this should be allowed, what should the xml format be?
It should be represented as multiple <source> tags in the XML. The fact that is multiplexes onto a single -soundhw arg is an implementation detail that should remain hidden.
Did you mean <sound> tags? Something like:
<sound model='d1'/> <sound model='d2'/>
yes, that's what i meant. Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
I was actually thinking about doing this very patch :-) . Are you going to also enable virt-manager to make use of it ? Since it's just good old qemu it will be great for local KVM use. Though it would be really nice if pulse audio support to where added to qemu. Then someway expose options also through libvirt on which audio subsystem to use (oss, alsa, or pulseaudio). Then with pulse audio you do have the ability to transport it over the net (though never tried it or set it up yet), to work with remote clients. On Mon, 2008-04-21 at 13:02 -0400, Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form:
<sound driver='drivername'/>
Where driver name can be pcspk, sb16, es1370, or all.
Everything seems to be in working order but I have a few implementation questions:
1) Should multiple drivers be able to be specified? qemu accommodates this, allowing '-soundhw sb16,pcspk' for example. If this should be allowed, what should the xml format be?
2) Should acceptable driver options be hardcoded? The other option is to just pass the input straight to qemu. This patch has the options hardcoded.
Also I realize this will probably need to be rediff'd around Dan's serial + parallel device patch, but I figured I would just get this out there.
Thanks, Cole plain text document attachment (libvirt-sound-patch) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index d9b82b2..bfd9ba4 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1011,6 +1011,41 @@ static int qemudParseInputXML(virConnectPtr conn, return -1; }
+/* Sound device helper functions */ +static int qemudSoundDriverFromString(virConnectPtr conn, + const char *driver) { + if (STREQ(driver, "all")) { + return QEMU_SOUND_ALL; + } else if (STREQ(driver, "sb16")) { + return QEMU_SOUND_SB16; + } else if (STREQ(driver, "es1370")) { + return QEMU_SOUND_ES1370; + } else if (STREQ(driver, "pcspk")) { + return QEMU_SOUND_PCSPK; + } + + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, + _("invalid sound driver '%s'"), driver); + return -1; +} + +static const char *qemudSoundDriverToString(virConnectPtr conn, + const int driver) { + + if (driver == QEMU_SOUND_ALL) { + return "all"; + } else if (driver == QEMU_SOUND_SB16) { + return "sb16"; + } else if (driver == QEMU_SOUND_ES1370) { + return "es1370"; + } else if (driver == QEMU_SOUND_PCSPK) { + return "pcspk"; + } + + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("invalid sound driver '%d'"), driver); + return NULL; +}
/* * Parses a libvirt XML definition of a guest, and populates the @@ -1486,6 +1521,25 @@ static struct qemud_vm_def *qemudParseXML(virConnectPtr conn, } } xmlXPathFreeObject(obj); + + /* Parse sound driver xml */ + obj = xmlXPathEval(BAD_CAST "/domain/devices/sound", ctxt); + if ((obj == NULL) || (obj->type != XPATH_NODESET) || + (obj->nodesetval == NULL) || (obj->nodesetval->nodeNr == 0)) { + def->soundDriver = QEMU_SOUND_NONE; + } else if ((prop = xmlGetProp(obj->nodesetval->nodeTab[0], + BAD_CAST "driver"))) { + + if ((def->soundDriver = qemudSoundDriverFromString(conn, + (char *) prop)) < 0) + goto error; + + xmlFree(prop); + prop = NULL; + } else { + def->soundDriver = QEMU_SOUND_NONE; + } + xmlXPathFreeObject(obj); obj = NULL;
/* If graphics are enabled, there's an implicit PS2 mouse */ @@ -1694,6 +1748,7 @@ int qemudBuildCommandLine(virConnectPtr conn, (vm->def->os.cmdline[0] ? 2 : 0) + /* cmdline */ (vm->def->graphicsType == QEMUD_GRAPHICS_VNC ? 2 : (vm->def->graphicsType == QEMUD_GRAPHICS_SDL ? 0 : 1)) + /* graphics */ + (vm->def->soundDriver == QEMU_SOUND_NONE ? 0 : 2) + /* sound */ (vm->migrateFrom[0] ? 3 : 0); /* migrateFrom */
snprintf(memory, sizeof(memory), "%lu", vm->def->memory/1024); @@ -1970,6 +2025,14 @@ int qemudBuildCommandLine(virConnectPtr conn, /* SDL is the default. no args needed */ }
+ /* Add sound hardware */ + if (vm->def->soundDriver != QEMU_SOUND_NONE) { + if (!((*argv)[++n] = strdup("-soundhw"))) + goto no_memory; + if (!((*argv)[++n] = strdup((char *) qemudSoundDriverToString(conn, vm->def->soundDriver)))) + goto no_memory; + } + if (vm->migrateFrom[0]) { if (!((*argv)[++n] = strdup("-S"))) goto no_memory; @@ -3125,7 +3188,11 @@ char *qemudGenerateXML(virConnectPtr conn, break; }
- if (def->graphicsType == QEMUD_GRAPHICS_VNC) { + if (def->soundDriver != QEMU_SOUND_NONE) { + if (virBufferVSprintf(buf, " <sound driver='%s'/>\n", + qemudSoundDriverToString(conn, + def->soundDriver)) < 0) + goto no_memory; }
if (virBufferAddLit(buf, " </devices>\n") < 0) diff --git a/src/qemu_conf.h b/src/qemu_conf.h index c59b1fa..1383c10 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -136,6 +136,14 @@ struct qemud_vm_input_def { struct qemud_vm_input_def *next; };
+enum qemu_vm_sound_driver { + QEMU_SOUND_NONE, + QEMU_SOUND_ALL, + QEMU_SOUND_SB16, + QEMU_SOUND_ES1370, + QEMU_SOUND_PCSPK, +}; + /* Flags for the 'type' field in next struct */ enum qemud_vm_device_type { QEMUD_DEVICE_DISK, @@ -214,6 +222,7 @@ struct qemud_vm_def { int vncActivePort; char vncListen[BR_INET_ADDR_MAXLEN]; char *keymap; + int soundDriver;
int ndisks; struct qemud_vm_disk_def *disks; diff --git a/src/xend_internal.c b/src/xend_internal.c index 6ba4571..b470731 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -1783,6 +1783,18 @@ xend_parse_sexp_desc(virConnectPtr conn, struct sexpr *root, } } } + + if (sexpr_node(root, "domain/image/hvm/soundhw")) { + tmp = sexpr_node(root, "domain/image/hvm/soundhw"); + if (tmp && *tmp) { + if (STREQ(tmp, "all") || + STREQ(tmp, "pcspk") || + STREQ(tmp, "sb16") || + STREQ(tmp, "es1370")) { + virBufferVSprintf(&buf, " <sound driver='%s'/>\n", tmp); + } + } + } }
/* Graphics device (HVM <= 3.0.4, or PV <= 3.0.3) vnc config */ diff --git a/src/xm_internal.c b/src/xm_internal.c index 3d845dc..91c7cf1 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -934,6 +934,15 @@ char *xenXMDomainFormatXML(virConnectPtr conn, virConfPtr conf) { /* Ignore else branch - probably some other non-input device we don't support in libvirt yet */ } + + if ((xenXMConfigGetString(conf, "soundhw", &str) == 0) && str) { + if (STREQ(str, "all") || + STREQ(str, "pcspk") || + STREQ(str, "sb16") || + STREQ(str, "es1370")) { + virBufferVSprintf(buf, " <sound driver='%s'/>\n", str); + } + } }
/* HVM guests, or old PV guests use this config format */ @@ -2081,6 +2090,11 @@ virConfPtr xenXMParseXMLToConfig(virConnectPtr conn, const char *xml) { if (xenXMConfigSetStringFromXPath(conn, conf, ctxt, "usbdevice", "string(/domain/devices/input[@bus='usb' or (not(@bus) and @type='tablet')]/@type)", 1, "cannot set the usbdevice parameter") < 0) goto error; + + if (xenXMConfigSetStringFromXPath(conn, conf, ctxt, "soundhw", + "string(/domain/devices/sound[@driver]", + 1, "cannot set soundhw parameter") < 0) + goto error; }
if (hvm || priv->xendConfigVersion < 3) { diff --git a/src/xml.c b/src/xml.c index 8e95103..f4bfb29 100644 --- a/src/xml.c +++ b/src/xml.c @@ -877,6 +877,14 @@ virDomainParseXMLOSDescHVM(virConnectPtr conn, xmlNodePtr node, nodes = NULL; }
+ cur = virXPathNode("/domain/devices/sound", ctxt); + if (cur) { + xmlChar *driver = NULL; + driver = xmlGetProp(cur, (xmlChar *) "driver"); + if (driver) + virBufferVSprintf(buf, "(soundhw '%s')", driver); + xmlFree(driver); + }
res = virXPathBoolean("count(domain/devices/console) > 0", ctxt); if (res < 0) { -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Jerone Young wrote:
I was actually thinking about doing this very patch :-) . Are you going to also enable virt-manager to make use of it ? Since it's just good old qemu it will be great for local KVM use.
Yeah, I'll probably start adding support for this in virt-install and virt-manager once this is committed.
Though it would be really nice if pulse audio support to where added to qemu. Then someway expose options also through libvirt on which audio subsystem to use (oss, alsa, or pulseaudio). Then with pulse audio you do have the ability to transport it over the net (though never tried it or set it up yet), to work with remote clients.
Sounds a little elaborate :) But hey, anythings a possibility as long as someone is willing to do the work. - Cole
Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form:
Here is the updated patch. Took a bit more work to take into account the multiple models, building and parsing the xen soundhw string, checking for duplicates, etc. The current version uses the format: <sound model='m1'/> <sound model='m2'/> To enable support for models m1 and m2. The code will fail if you attempt to define an xml config which specifies a model that isn't in the whitelist (currently composed of 'sb16, 'es1370', and 'pcspk'). Unknown values from a xend sexpr or config file will be silently ignored. One question: should the value 'all' be recognized from a xen domain and translated into a <sound> tag for every item in the whitelist? 'all' is an accepted value for a xen domain, since it just passes the string to qemu. This isn't in the code but I only thought of it now. Again, this needs to be rediff'd around recent commits (virBuffer changes, probably others), which I will do next round after any feedback. Thanks, Cole diff --git a/src/qemu_conf.c b/src/qemu_conf.c index d9b82b2..1b68806 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1011,6 +1011,64 @@ static int qemudParseInputXML(virConnectPtr conn, return -1; } +/* Sound device helper functions */ +static int qemudSoundModelFromString(virConnectPtr conn, + const char *model) { + if (STREQ(model, "sb16")) { + return QEMU_SOUND_SB16; + } else if (STREQ(model, "es1370")) { + return QEMU_SOUND_ES1370; + } else if (STREQ(model, "pcspk")) { + return QEMU_SOUND_PCSPK; + } + + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, + _("invalid sound model '%s'"), model); + return -1; +} + +static const char *qemudSoundModelToString(virConnectPtr conn, + const int model) { + + if (model == QEMU_SOUND_SB16) { + return "sb16"; + } else if (model == QEMU_SOUND_ES1370) { + return "es1370"; + } else if (model == QEMU_SOUND_PCSPK) { + return "pcspk"; + } + + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("invalid sound model '%d'"), model); + return NULL; +} + + +static int qemudParseSoundXML(virConnectPtr conn, + struct qemud_vm_sound_def *sound, + xmlNodePtr node) { + + xmlChar *model = NULL; + model = xmlGetProp(node, BAD_CAST "model"); + + if (!model) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("missing sound model")); + goto error; + } + if ((sound->model = qemudSoundModelFromString(conn, (char *) model)) < 0) + goto error; + + if (model) + xmlFree(model); + return 0; + + error: + if (model) + xmlFree(model); + return -1; +} + /* * Parses a libvirt XML definition of a guest, and populates the @@ -1486,6 +1544,50 @@ static struct qemud_vm_def *qemudParseXML(virConnectPtr conn, } } xmlXPathFreeObject(obj); + + /* Parse sound driver xml */ + obj = xmlXPathEval(BAD_CAST "/domain/devices/sound", ctxt); + if ((obj != NULL) && (obj->type == XPATH_NODESET) && + (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr >= 0)) { + struct qemud_vm_sound_def *prev = NULL; + for (i = 0; i < obj->nodesetval->nodeNr; i++) { + + struct qemud_vm_sound_def *sound = calloc(1, sizeof(*sound)); + struct qemud_vm_sound_def *check = def->sounds; + int collision = 0; + if (!sound) { + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, + "%s", _("failed to allocate space for sound dev")); + goto error; + } + if (qemudParseSoundXML(conn, sound, + obj->nodesetval->nodeTab[i]) < 0) { + free(sound); + goto error; + } + + // Check that model type isn't already present in sound dev list + while(check) { + if (check->model == sound->model) { + collision = 1; + break; + } + check = check->next; + } + if (collision) + continue; + + def->nsounds++; + sound->next = NULL; + if (def->sounds == NULL) { + def->sounds = sound; + } else { + prev->next = sound; + } + prev = sound; + } + } + xmlXPathFreeObject(obj); obj = NULL; /* If graphics are enabled, there's an implicit PS2 mouse */ @@ -1633,6 +1735,7 @@ int qemudBuildCommandLine(virConnectPtr conn, struct qemud_vm_disk_def *disk = vm->def->disks; struct qemud_vm_net_def *net = vm->def->nets; struct qemud_vm_input_def *input = vm->def->inputs; + struct qemud_vm_sound_def *sound = vm->def->sounds; struct utsname ut; int disableKQEMU = 0; @@ -1681,6 +1784,7 @@ int qemudBuildCommandLine(virConnectPtr conn, (vm->def->nnets > 0 ? (4 * vm->def->nnets) : 2) + /* networks */ 1 + /* usb */ 2 * vm->def->ninputs + /* input devices */ + ((vm->def->nsounds > 0) ? 2 : 0) + /* sound */ 2 + /* memory*/ 2 + /* cpus */ 2 + /* boot device */ @@ -1970,6 +2074,27 @@ int qemudBuildCommandLine(virConnectPtr conn, /* SDL is the default. no args needed */ } + /* Add sound hardware */ + if (sound) { + int size = 100; + char *modstr = calloc(1, size+1); + if (!modstr) + goto no_memory; + if (!((*argv)[++n] = strdup("-soundhw"))) + goto no_memory; + + while(sound && size > 0) { + const char *model = qemudSoundModelToString(conn, sound->model); + strncat(modstr, model, size); + size -= strlen(model); + sound = sound->next; + if (sound) + strncat(modstr, ",", size--); + } + if (!((*argv)[++n] = modstr)) + goto no_memory; + } + if (vm->migrateFrom[0]) { if (!((*argv)[++n] = strdup("-S"))) goto no_memory; @@ -2081,6 +2206,9 @@ qemudParseVMDeviceDef(virConnectPtr conn, } else if (xmlStrEqual(node->name, BAD_CAST "input")) { dev->type = QEMUD_DEVICE_DISK; qemudParseInputXML(conn, &(dev->data.input), node); + } else if (xmlStrEqual(node->name, BAD_CAST "sound")) { + dev->type = QEMUD_DEVICE_SOUND; + qemudParseSoundXML(conn, &(dev->data.sound), node); } else { qemudReportError(conn, NULL, NULL, VIR_ERR_XML_ERROR, "%s", _("unknown device type")); @@ -2850,6 +2978,7 @@ char *qemudGenerateXML(virConnectPtr conn, struct qemud_vm_disk_def *disk; struct qemud_vm_net_def *net; struct qemud_vm_input_def *input; + struct qemud_vm_sound_def *sound; const char *type = NULL; int n; @@ -3125,7 +3254,12 @@ char *qemudGenerateXML(virConnectPtr conn, break; } - if (def->graphicsType == QEMUD_GRAPHICS_VNC) { + sound = def->sounds; + while(sound) { + if (virBufferVSprintf(buf, " <sound model='%s'/>\n", + qemudSoundModelToString(conn, sound->model)) < 0) + goto no_memory; + sound = sound->next; } if (virBufferAddLit(buf, " </devices>\n") < 0) diff --git a/src/qemu_conf.h b/src/qemu_conf.h index c59b1fa..b9b1ca5 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -136,11 +136,24 @@ struct qemud_vm_input_def { struct qemud_vm_input_def *next; }; +enum qemu_vm_sound_model { + QEMU_SOUND_NONE = 0, + QEMU_SOUND_SB16, + QEMU_SOUND_ES1370, + QEMU_SOUND_PCSPK, +}; + +struct qemud_vm_sound_def { + int model; + struct qemud_vm_sound_def *next; +}; + /* Flags for the 'type' field in next struct */ enum qemud_vm_device_type { QEMUD_DEVICE_DISK, QEMUD_DEVICE_NET, QEMUD_DEVICE_INPUT, + QEMUD_DEVICE_SOUND, }; struct qemud_vm_device_def { @@ -149,6 +162,7 @@ struct qemud_vm_device_def { struct qemud_vm_disk_def disk; struct qemud_vm_net_def net; struct qemud_vm_input_def input; + struct qemud_vm_sound_def sound; } data; }; @@ -223,6 +237,9 @@ struct qemud_vm_def { int ninputs; struct qemud_vm_input_def *inputs; + + int nsounds; + struct qemud_vm_sound_def *sounds; }; /* Guest VM runtime state */ diff --git a/src/xend_internal.c b/src/xend_internal.c index 6ba4571..eff7653 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -852,6 +852,69 @@ urlencode(const char *string) return buffer; } + +/** + * sound_string_to_xml: + * @soundstr : soundhw string for the form m1,m2,m3 ... + * + * Parses the passed string and returns a heap allocated string containing + * the valid libvirt soundxml. Must be free'd by caller. + * + * Returns NULL on fail, xml string on success (can be the empty string). + */ +char *sound_string_to_xml(const char *sound) { + + char *comma, *model, *dupe; + virBuffer buf; + int collision, modelsize; + + if (!(buf.content = calloc(1, 1024))) + return NULL; + buf.size = 1024; + buf.use = 0; + + while (sound) { + + collision = 0; + model = NULL; + modelsize = strlen(sound); + if ((comma = strchr(sound, ','))) { + modelsize -= strlen(comma); + } + + // Parse out first element up to comma + if (!strncmp(sound, "sb16", modelsize)) { + model = strdup("sb16"); + } else if (!strncmp(sound, "es1370", modelsize)) { + model = strdup("es1370"); + } else if (!strncmp(sound, "pcspk", modelsize)) { + model = strdup("pcspk"); + } + + // Check that model is not already in remaining soundstr + if (comma && model && (dupe = strstr(comma, model))) { + if (( (dupe == sound) || //(Start of line | + (*(dupe - 1) == ',') ) && // Preceded by comma) & + ( (dupe[strlen(model)] == ',') || //(Ends with comma | + (dupe[strlen(model)] == '\0') )) // Ends whole string) + collision = 1; + } + + if (!collision && + virBufferVSprintf(&buf, " <sound model='%s'/>\n", model)) { + free(model); + return NULL; + } + + sound = comma; + if (comma) + sound++; + free(model); + } + + return buf.content; +} + #endif /* ! PROXY */ /* PUBLIC FUNCTIONS */ @@ -1783,6 +1846,21 @@ xend_parse_sexp_desc(virConnectPtr conn, struct sexpr *root, } } } + + if (sexpr_node(root, "domain/image/hvm/soundhw")) { + char *soundxml; + tmp = sexpr_node(root, "domain/image/hvm/soundhw"); + if (tmp && *tmp) { + if ((soundxml = sound_string_to_xml(tmp))) { + virBufferVSprintf(&buf, "%s", soundxml); + free(soundxml); + } else { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("parsing soundhw string failed.")); + goto error; + } + } + } } /* Graphics device (HVM <= 3.0.4, or PV <= 3.0.3) vnc config */ diff --git a/src/xend_internal.h b/src/xend_internal.h index e157e88..377b67b 100644 --- a/src/xend_internal.h +++ b/src/xend_internal.h @@ -181,6 +181,7 @@ char *xenDaemonDomainDumpXMLByName(virConnectPtr xend, int xend_log(virConnectPtr xend, char *buffer, size_t n_buffer); char *xend_parse_domain_sexp(virConnectPtr conn, char *root, int xendConfigVersion); + char *sound_string_to_xml(const char *sound); /* refactored ones */ int xenDaemonOpen(virConnectPtr conn, xmlURIPtr uri, virConnectAuthPtr auth, int flags); diff --git a/src/xm_internal.c b/src/xm_internal.c index 3d845dc..1ef0746 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -934,6 +934,18 @@ char *xenXMDomainFormatXML(virConnectPtr conn, virConfPtr conf) { /* Ignore else branch - probably some other non-input device we don't support in libvirt yet */ } + + if ((xenXMConfigGetString(conf, "soundhw", &str) == 0) && str) { + char *soundxml; + if ((soundxml = sound_string_to_xml(str))) { + virBufferVSprintf(buf, "%s", soundxml); + free(soundxml); + } else { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("parsing soundhw string failed.")); + goto error; + } + } } /* HVM guests, or old PV guests use this config format */ @@ -1040,6 +1052,10 @@ char *xenXMDomainFormatXML(virConnectPtr conn, virConfPtr conf) { buf->content = NULL; virBufferFree(buf); return (xml); + + error: + virBufferFree(buf); + return (NULL); } @@ -2081,6 +2097,17 @@ virConfPtr xenXMParseXMLToConfig(virConnectPtr conn, const char *xml) { if (xenXMConfigSetStringFromXPath(conn, conf, ctxt, "usbdevice", "string(/domain/devices/input[@bus='usb' or (not(@bus) and @type='tablet')]/@type)", 1, "cannot set the usbdevice parameter") < 0) goto error; + + if (virXPathNode("/domain/devices/sound", ctxt)) { + char *soundstr; + if (!(soundstr = virBuildSoundStringFromXML(conn, ctxt))) + goto error; + if (xenXMConfigSetString(conf, "soundhw", soundstr) < 0) { + free(soundstr); + goto error; + } + free(soundstr); + } } if (hvm || priv->xendConfigVersion < 3) { diff --git a/src/xml.c b/src/xml.c index 8e95103..3bd51f1 100644 --- a/src/xml.c +++ b/src/xml.c @@ -289,6 +289,85 @@ virConvertCpuSet(virConnectPtr conn, const char *str, int maxcpu) { free(cpuset); return (res); } + +/** + * virBuildSoundStringFromXML + * @sound buffer to populate + * @len size of preallocated buffer 'sound' + * @ctxt xml context to pull sound info from + * + * Builds a string of the form m1,m2,m3 from the different sound models + * in the xml. String must be free'd by caller. + * + * Returns string on success, NULL on error + */ +char * virBuildSoundStringFromXML(virConnectPtr conn, + xmlXPathContextPtr ctxt) { + + int nb_nodes, size = 256; + char *dupe, *sound; + xmlNodePtr *nodes = NULL; + + if (!(sound = calloc(1, size+1))) { + virXMLError(conn, VIR_ERR_NO_MEMORY, + _("failed to allocate sound string"), 0); + return NULL; + } + + nb_nodes = virXPathNodeSet("/domain/devices/sound", ctxt, &nodes); + if (nb_nodes > 0) { + int i; + for (i = 0; i < nb_nodes && size > 0; i++) { + char *model = NULL; + int collision = 0; + + model = (char *) xmlGetProp(nodes[i], (xmlChar *) "model"); + if (!model) { + virXMLError(conn, VIR_ERR_XML_ERROR, + _("no model for sound device"), 0); + goto error; + } + + if (!(STREQ(model, "pcspk")|| + STREQ(model, "sb16") || + STREQ(model, "es1370"))) { + virXMLError(conn, VIR_ERR_XML_ERROR, + _("unknown sound model type"), 0); + free(model); + goto error; + } + + // Check for duplicates in currently built string + if (*sound && (dupe = strstr(sound, model))) { + if (( (dupe == sound) || //(Start of line | + (*(dupe - 1) == ',') ) && // Preceded by comma) & + ( (dupe[strlen(model)] == ',') || //(Ends with comma | + (dupe[strlen(model)] == '\0') )) // Ends whole string) + collision = 1; + } + + // If no collision, add to string + if (!collision) { + if (*sound && (size >= (strlen(model) + 1))) { + strncat(sound, ",", size--); + } else if (*sound || size < strlen(model)) { + free(model); + continue; + } + strncat(sound, model, size); + size -= strlen(model); + } + + free(model); + } + } + free(nodes); + return sound; + + error: + free(nodes); + return NULL; +} #endif /* WITH_XEN */ #ifndef PROXY @@ -877,6 +956,14 @@ virDomainParseXMLOSDescHVM(virConnectPtr conn, xmlNodePtr node, nodes = NULL; } + cur = virXPathNode("/domain/devices/sound", ctxt); + if (cur) { + char *soundstr; + if (!(soundstr = virBuildSoundStringFromXML(conn, ctxt))) + goto error; + virBufferVSprintf(buf, "(soundhw '%s')", soundstr); + free(soundstr); + } res = virXPathBoolean("count(domain/devices/console) > 0", ctxt); if (res < 0) { diff --git a/src/xml.h b/src/xml.h index 2d30b65..f9a0e5b 100644 --- a/src/xml.h +++ b/src/xml.h @@ -57,6 +57,8 @@ int virDomainXMLDevID(virDomainPtr domain, char *class, char *ref, int ref_len); +char * virBuildSoundStringFromXML(virConnectPtr conn, + xmlXPathContextPtr ctxt); #endif #ifdef __cplusplus
On Mon, Apr 28, 2008 at 01:44:32PM -0400, Cole Robinson wrote:
Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form:
Here is the updated patch. Took a bit more work to take into account the multiple models, building and parsing the xen soundhw string, checking for duplicates, etc.
The current version uses the format:
<sound model='m1'/> <sound model='m2'/>
To enable support for models m1 and m2. The code will fail if you attempt to define an xml config which specifies a model that isn't in the whitelist (currently composed of 'sb16, 'es1370', and 'pcspk'). Unknown values from a xend sexpr or config file will be silently ignored.
One question: should the value 'all' be recognized from a xen domain and translated into a <sound> tag for every item in the whitelist? 'all' is an accepted value for a xen domain, since it just passes the string to qemu. This isn't in the code but I only thought of it now.
Sigh, that's a good question. I think I'd probably turn it into a list of 3 <sound> tags, so that if they load the XML back in, it'll get changed to the canonical explicit format.
+static int qemudParseSoundXML(virConnectPtr conn, + struct qemud_vm_sound_def *sound, + xmlNodePtr node) { + + xmlChar *model = NULL; + model = xmlGetProp(node, BAD_CAST "model"); + + if (!model) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("missing sound model")); + goto error; + } + if ((sound->model = qemudSoundModelFromString(conn, (char *) model)) < 0) + goto error; + + if (model) + xmlFree(model); + return 0; + + error: + if (model) + xmlFree(model); + return -1; +}
IIRC, xmlFree already copes with NULL, the 'if()' bits are not required. You can check with 'make syntax-check' and see if it complains about these lines. Aside from this, it looks reasonable - I'd like to see a couple of XML configs added to the test cases for xen and QEMU to validate the parsing and formatting of XML. Regards, Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Cole Robinson <crobinso@redhat.com> wrote:
Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form: ... Again, this needs to be rediff'd around recent commits (virBuffer changes, probably others), which I will do next round after any feedback.
Hi Cole, Yes, parsing in C is no fun...
diff --git a/src/qemu_conf.c b/src/qemu_conf.c index d9b82b2..1b68806 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1011,6 +1011,64 @@ static int qemudParseInputXML(virConnectPtr conn, return -1; }
+/* Sound device helper functions */ +static int qemudSoundModelFromString(virConnectPtr conn, + const char *model) {
Please don't require a "conn" argument in functions like this. Instead, let the caller handle any diagnostic. For an example where this would be useful, see below.
+ if (STREQ(model, "sb16")) { + return QEMU_SOUND_SB16; + } else if (STREQ(model, "es1370")) { + return QEMU_SOUND_ES1370; + } else if (STREQ(model, "pcspk")) { + return QEMU_SOUND_PCSPK; + } + + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, + _("invalid sound model '%s'"), model); + return -1; +} + +static const char *qemudSoundModelToString(virConnectPtr conn, + const int model) {
Likewise.
+ if (model == QEMU_SOUND_SB16) { + return "sb16"; + } else if (model == QEMU_SOUND_ES1370) { + return "es1370"; + } else if (model == QEMU_SOUND_PCSPK) { + return "pcspk"; + } + + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("invalid sound model '%d'"), model); + return NULL; +} + + +static int qemudParseSoundXML(virConnectPtr conn, + struct qemud_vm_sound_def *sound, + xmlNodePtr node) {
Can this be "const"? It looks like it should be.
+ const xmlNodePtr node) {
Maybe omit "conn" here too. Maybe. Callers could say "missing or invalid sound model", but that's slightly degraded from the current "invalid sound model '%s'".
+ xmlChar *model = NULL; + model = xmlGetProp(node, BAD_CAST "model"); + + if (!model) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("missing sound model")); + goto error; + } + if ((sound->model = qemudSoundModelFromString(conn, (char *) model)) < 0) + goto error; + + if (model) + xmlFree(model); + return 0; + + error: + if (model) + xmlFree(model); + return -1;
It's better to record "int err = -1;" initially, and set err = 0 if all goes well. Then you can have a single point of return and avoid the duplicate xmlFree, e.g., err = 0; error: xmlFree(model); return err; ...
+ /* Add sound hardware */ + if (sound) { + int size = 100; + char *modstr = calloc(1, size+1); + if (!modstr) + goto no_memory; + if (!((*argv)[++n] = strdup("-soundhw"))) + goto no_memory; + + while(sound && size > 0) { + const char *model = qemudSoundModelToString(conn, sound->model); + strncat(modstr, model, size);
qemudSoundModelToString can return NULL, so you won't want to use strncat or strlen on that. ...
+char *sound_string_to_xml(const char *sound) { + + char *comma, *model, *dupe; + virBuffer buf; + int collision, modelsize;
Most of the above (but e.g., not "buf") declarations can be moved "down" into the scope where they're used.
+ if (!(buf.content = calloc(1, 1024))) + return NULL; + buf.size = 1024; + buf.use = 0; + + while (sound) { + + collision = 0; + model = NULL; + modelsize = strlen(sound); + if ((comma = strchr(sound, ','))) { + modelsize -= strlen(comma); + }
slightly more efficient, and clearer to me: comma = strchr(sound, ','); modelsize = comma ? comma - sound : strlen (sound);
+ + // Parse out first element up to comma + if (!strncmp(sound, "sb16", modelsize)) { + model = strdup("sb16"); + } else if (!strncmp(sound, "es1370", modelsize)) { + model = strdup("es1370"); + } else if (!strncmp(sound, "pcspk", modelsize)) { + model = strdup("pcspk"); + }
You can avoid enumerating the types here. E.g., model = strndup(sound, modelsize); if (!model) continue; if ((m = qemudSoundModelFromString(model)) < 0) { free(model); continue; } The duplicate detection below misses when a real match is obscured by a partial one, e.g., a string like "es1370.,es1370" where strstr matches, but a subsequent word-boundary test fails. How about a cheaper test, e.g., (with this declaration and initialization above: char seenSoundModel[20]; memset(seenSoundModel,0,sizeof seenSoundModel); assert (m < sizeof seenSoundModel); if (seenSoundModel[m]) collision = 1; seenSoundModel[m] = 1;
+ // Check that model is not already in remaining soundstr + if (comma && model && (dupe = strstr(comma, model))) { + if (( (dupe == sound) || //(Start of line | + (*(dupe - 1) == ',') ) && // Preceded by comma) & + ( (dupe[strlen(model)] == ',') || //(Ends with comma | + (dupe[strlen(model)] == '\0') )) // Ends whole string) + collision = 1; + } ... /* HVM guests, or old PV guests use this config format */ @@ -1040,6 +1052,10 @@ char *xenXMDomainFormatXML(virConnectPtr conn, virConfPtr conf) { buf->content = NULL; virBufferFree(buf); return (xml); + + error: + virBufferFree(buf); + return (NULL);
You can avoid this duplication, too. As suggested above except here with "char *ret = NULL;", etc. and "return xml;"
} ... +char * virBuildSoundStringFromXML(virConnectPtr conn, + xmlXPathContextPtr ctxt) { + + int nb_nodes, size = 256; + char *dupe, *sound; + xmlNodePtr *nodes = NULL; + + if (!(sound = calloc(1, size+1))) { + virXMLError(conn, VIR_ERR_NO_MEMORY, + _("failed to allocate sound string"), 0); + return NULL; + } + + nb_nodes = virXPathNodeSet("/domain/devices/sound", ctxt, &nodes); + if (nb_nodes > 0) { + int i; + for (i = 0; i < nb_nodes && size > 0; i++) { + char *model = NULL; + int collision = 0; + + model = (char *) xmlGetProp(nodes[i], (xmlChar *) "model"); + if (!model) { + virXMLError(conn, VIR_ERR_XML_ERROR, + _("no model for sound device"), 0); + goto error; + } + + if (!(STREQ(model, "pcspk")|| + STREQ(model, "sb16") || + STREQ(model, "es1370"))) {
Don't repeat this list of literal strings. Imagine having to add a new model type. You'll want that change to require modification of a minimum number of places in the code (probably just 2: convert from string to int and int to string). Instead, just test qemudSoundModelFromString(model) < 0
+ virXMLError(conn, VIR_ERR_XML_ERROR, + _("unknown sound model type"), 0); + free(model); + goto error; + } + + // Check for duplicates in currently built string + if (*sound && (dupe = strstr(sound, model))) {
This code looks way too similar to the dup-detecting code above. Once the above is in a shape you like, consider factoring it out into a function and reusing that function here.
+ if (( (dupe == sound) || //(Start of line | + (*(dupe - 1) == ',') ) && // Preceded by comma) & + ( (dupe[strlen(model)] == ',') || //(Ends with comma | + (dupe[strlen(model)] == '\0') )) // Ends whole string) + collision = 1; + } + + // If no collision, add to string + if (!collision) { + if (*sound && (size >= (strlen(model) + 1))) { + strncat(sound, ",", size--); + } else if (*sound || size < strlen(model)) { + free(model); + continue; + } + strncat(sound, model, size); + size -= strlen(model); ...
Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen.
Third iteration. Changes include: - Rediffd against current code - Reading 'all' from a xend sexpr or xm config does the right thing. - Added test files. - Addressed most of Jim's feedback. Unfortunately I could not reuse the qemu sound functions, as xen (on f8 at least) doesn't support the pcspk model, so the whitelist isn't the same between the two. Any (more) feedback is appreciated. Thanks, Cole diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 1efd9e9..4f5e96e 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1373,6 +1373,58 @@ static int qemudParseInputXML(virConnectPtr conn, return -1; } +/* Sound device helper functions */ +static int qemudSoundModelFromString(const char *model) { + if (STREQ(model, "sb16")) { + return QEMU_SOUND_SB16; + } else if (STREQ(model, "es1370")) { + return QEMU_SOUND_ES1370; + } else if (STREQ(model, "pcspk")) { + return QEMU_SOUND_PCSPK; + } + + return -1; +} + +static const char *qemudSoundModelToString(const int model) { + + if (model == QEMU_SOUND_SB16) { + return "sb16"; + } else if (model == QEMU_SOUND_ES1370) { + return "es1370"; + } else if (model == QEMU_SOUND_PCSPK) { + return "pcspk"; + } + + return NULL; +} + + +static int qemudParseSoundXML(virConnectPtr conn, + struct qemud_vm_sound_def *sound, + const xmlNodePtr node) { + + int err = -1; + xmlChar *model = NULL; + model = xmlGetProp(node, BAD_CAST "model"); + + if (!model) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("missing sound model")); + goto error; + } + if ((sound->model = qemudSoundModelFromString((char *) model)) < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("invalid sound model '%s'"), (char *) model); + goto error; + } + + err = 0; + error: + xmlFree(model); + return err; +} + /* * Parses a libvirt XML definition of a guest, and populates the @@ -1887,6 +1939,50 @@ static struct qemud_vm_def *qemudParseXML(virConnectPtr conn, } } xmlXPathFreeObject(obj); + + /* Parse sound driver xml */ + obj = xmlXPathEval(BAD_CAST "/domain/devices/sound", ctxt); + if ((obj != NULL) && (obj->type == XPATH_NODESET) && + (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr >= 0)) { + struct qemud_vm_sound_def *prev = NULL; + for (i = 0; i < obj->nodesetval->nodeNr; i++) { + + struct qemud_vm_sound_def *sound = calloc(1, sizeof(*sound)); + struct qemud_vm_sound_def *check = def->sounds; + int collision = 0; + if (!sound) { + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, + "%s", _("failed to allocate space for sound dev")); + goto error; + } + if (qemudParseSoundXML(conn, sound, + obj->nodesetval->nodeTab[i]) < 0) { + free(sound); + goto error; + } + + // Check that model type isn't already present in sound dev list + while(check) { + if (check->model == sound->model) { + collision = 1; + break; + } + check = check->next; + } + if (collision) + continue; + + def->nsounds++; + sound->next = NULL; + if (def->sounds == NULL) { + def->sounds = sound; + } else { + prev->next = sound; + } + prev = sound; + } + } + xmlXPathFreeObject(obj); obj = NULL; /* If graphics are enabled, there's an implicit PS2 mouse */ @@ -2106,6 +2202,7 @@ int qemudBuildCommandLine(virConnectPtr conn, struct qemud_vm_disk_def *disk = vm->def->disks; struct qemud_vm_net_def *net = vm->def->nets; struct qemud_vm_input_def *input = vm->def->inputs; + struct qemud_vm_sound_def *sound = vm->def->sounds; struct qemud_vm_chr_def *serial = vm->def->serials; struct qemud_vm_chr_def *parallel = vm->def->parallels; struct utsname ut; @@ -2156,6 +2253,7 @@ int qemudBuildCommandLine(virConnectPtr conn, (vm->def->nnets > 0 ? (4 * vm->def->nnets) : 2) + /* networks */ 1 + /* usb */ 2 * vm->def->ninputs + /* input devices */ + ((vm->def->nsounds > 0) ? 2 : 0) + /* sound */ (vm->def->nserials > 0 ? (2 * vm->def->nserials) : 2) + /* character devices */ (vm->def->nparallels > 0 ? (2 * vm->def->nparallels) : 2) + /* character devices */ 2 + /* memory*/ @@ -2491,6 +2589,32 @@ int qemudBuildCommandLine(virConnectPtr conn, /* SDL is the default. no args needed */ } + /* Add sound hardware */ + if (sound) { + int size = 100; + char *modstr = calloc(1, size+1); + if (!modstr) + goto no_memory; + if (!((*argv)[++n] = strdup("-soundhw"))) + goto no_memory; + + while(sound && size > 0) { + const char *model = qemudSoundModelToString(sound->model); + if (!model) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("invalid sound model")); + goto error; + } + strncat(modstr, model, size); + size -= strlen(model); + sound = sound->next; + if (sound) + strncat(modstr, ",", size--); + } + if (!((*argv)[++n] = modstr)) + goto no_memory; + } + if (vm->migrateFrom[0]) { if (!((*argv)[++n] = strdup("-S"))) goto no_memory; @@ -2602,6 +2726,9 @@ qemudParseVMDeviceDef(virConnectPtr conn, } else if (xmlStrEqual(node->name, BAD_CAST "input")) { dev->type = QEMUD_DEVICE_DISK; qemudParseInputXML(conn, &(dev->data.input), node); + } else if (xmlStrEqual(node->name, BAD_CAST "sound")) { + dev->type = QEMUD_DEVICE_SOUND; + qemudParseSoundXML(conn, &(dev->data.sound), node); } else { qemudReportError(conn, NULL, NULL, VIR_ERR_XML_ERROR, "%s", _("unknown device type")); @@ -3475,6 +3602,7 @@ char *qemudGenerateXML(virConnectPtr conn, const struct qemud_vm_disk_def *disk; const struct qemud_vm_net_def *net; const struct qemud_vm_input_def *input; + const struct qemud_vm_sound_def *sound; const struct qemud_vm_chr_def *chr; const char *type = NULL; int n; @@ -3717,6 +3845,18 @@ char *qemudGenerateXML(virConnectPtr conn, break; } + sound = def->sounds; + while(sound) { + const char *model = qemudSoundModelToString(sound->model); + if (!model) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("invalid sound model")); + goto cleanup; + } + virBufferVSprintf(&buf, " <sound model='%s'/>\n", model); + sound = sound->next; + } + virBufferAddLit(&buf, " </devices>\n"); virBufferAddLit(&buf, "</domain>\n"); diff --git a/src/qemu_conf.h b/src/qemu_conf.h index bd8d8b2..46c6fb7 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -186,11 +186,24 @@ struct qemud_vm_input_def { struct qemud_vm_input_def *next; }; +enum qemu_vm_sound_model { + QEMU_SOUND_NONE = 0, + QEMU_SOUND_SB16, + QEMU_SOUND_ES1370, + QEMU_SOUND_PCSPK, +}; + +struct qemud_vm_sound_def { + int model; + struct qemud_vm_sound_def *next; +}; + /* Flags for the 'type' field in next struct */ enum qemud_vm_device_type { QEMUD_DEVICE_DISK, QEMUD_DEVICE_NET, QEMUD_DEVICE_INPUT, + QEMUD_DEVICE_SOUND, }; struct qemud_vm_device_def { @@ -199,6 +212,7 @@ struct qemud_vm_device_def { struct qemud_vm_disk_def disk; struct qemud_vm_net_def net; struct qemud_vm_input_def input; + struct qemud_vm_sound_def sound; } data; }; @@ -274,6 +288,9 @@ struct qemud_vm_def { unsigned int ninputs; struct qemud_vm_input_def *inputs; + unsigned int nsounds; + struct qemud_vm_sound_def *sounds; + unsigned int nserials; struct qemud_vm_chr_def *serials; diff --git a/src/xend_internal.c b/src/xend_internal.c index 29a16dd..0817b7d 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -851,6 +851,107 @@ urlencode(const char *string) return buffer; } + +/* Applicable sound models */ +const char *sound_models[] = { "sb16", "es1370" }; + +/** + * is_sound_model_valid: + * @model : model string to check against whitelist + * + * checks passed model string against whitelist of acceptable models + * + * Returns 0 if invalid, 1 otherwise + */ +int is_sound_model_valid(const char *model) { + int i; + + for (i = 0; i < sizeof(sound_models)/sizeof(*sound_models); ++i) { + if (STREQ(model, sound_models[i])) { + return 1; + } + } + return 0; +} + +/** + * is_sound_model_conflict: + * @model : model string to look for duplicates of + * @soundstr : soundhw string for the form m1,m2,m3 ... + * + * Returns 0 if no conflict, 1 otherwise + */ +int is_sound_model_conflict(const char *model, const char *soundstr) { + + char *dupe; + char *cur = (char *) soundstr; + while ((dupe = strstr(cur, model))) { + if (( (dupe == cur) || // (Start of line | + (*(dupe - 1) == ',') ) && // Preceded by comma) & + ( (dupe[strlen(model)] == ',') || // (Ends with comma | + (dupe[strlen(model)] == '\0') )) // Ends whole string) + return 1; + else + cur = dupe + strlen(model); + } + return 0; +} + +/** + * sound_string_to_xml: + * @soundstr : soundhw string for the form m1,m2,m3 ... + * + * Parses the passed string and returns a heap allocated string containing + * the valid libvirt soundxml. Must be free'd by caller. + * + * Returns NULL on fail, xml string on success (can be the empty string). + */ +char *sound_string_to_xml(const char *sound) { + + virBuffer buf = VIR_BUFFER_INITIALIZER; + + while (sound) { + int modelsize, valid, collision = 0; + char *model = NULL; + char *model_end = strchr(sound, ','); + modelsize = (model_end ? (model_end - sound) : strlen(sound)); + + if(!(model = strndup(sound, modelsize))) + goto error; + + if (!(valid = is_sound_model_valid(model))) { + // Check for magic 'all' model. If found, throw out current xml + // and build with all available models + if (STREQ(model, "all")) { + int i; + free(virBufferContentAndReset(&buf)); + + for (i=0; i < sizeof(sound_models)/sizeof(*sound_models); ++i) + virBufferVSprintf(&buf, " <sound model='%s'/>\n", + sound_models[i]); + free(model); + break; + } + } + + if (valid && model_end) + collision = is_sound_model_conflict(model, model_end); + if (valid && !collision) + virBufferVSprintf(&buf, " <sound model='%s'/>\n", model); + + sound = (model_end ? ++model_end : NULL); + free(model); + } + + if (virBufferError(&buf)) + goto error; + return virBufferContentAndReset(&buf); + + error: + free(virBufferContentAndReset(&buf)); + return NULL; +} + #endif /* ! PROXY */ /* PUBLIC FUNCTIONS */ @@ -2079,6 +2180,23 @@ xend_parse_sexp_desc(virConnectPtr conn, struct sexpr *root, } free(tty); + if (hvm) { + if (sexpr_node(root, "domain/image/hvm/soundhw")) { + char *soundxml; + tmp = sexpr_node(root, "domain/image/hvm/soundhw"); + if (tmp && *tmp) { + if ((soundxml = sound_string_to_xml(tmp))) { + virBufferVSprintf(&buf, "%s", soundxml); + free(soundxml); + } else { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("parsing soundhw string failed.")); + goto error; + } + } + } + } + virBufferAddLit(&buf, " </devices>\n"); virBufferAddLit(&buf, "</domain>\n"); diff --git a/src/xend_internal.h b/src/xend_internal.h index d56d73d..80ef4f6 100644 --- a/src/xend_internal.h +++ b/src/xend_internal.h @@ -189,6 +189,11 @@ char *xenDaemonDomainDumpXMLByName(virConnectPtr xend, char *xend_parse_domain_sexp(virConnectPtr conn, char *root, int xendConfigVersion); + int is_sound_model_valid(const char *model); + int is_sound_model_conflict(const char *model, const char *soundstr); + char *sound_string_to_xml(const char *sound); + + /* refactored ones */ int xenDaemonOpen(virConnectPtr conn, xmlURIPtr uri, virConnectAuthPtr auth, int flags); int xenDaemonClose(virConnectPtr conn); diff --git a/src/xm_internal.c b/src/xm_internal.c index 815f08b..c820c53 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1050,16 +1050,34 @@ char *xenXMDomainFormatXML(virConnectPtr conn, virConfPtr conf) { virBufferAddLit(&buf, " </console>\n"); } + if (hvm) { + if ((xenXMConfigGetString(conf, "soundhw", &str) == 0) && str) { + char *soundxml; + if ((soundxml = sound_string_to_xml(str))) { + virBufferVSprintf(&buf, "%s", soundxml); + free(soundxml); + } else { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("parsing soundhw string failed.")); + goto error; + } + } + } + virBufferAddLit(&buf, " </devices>\n"); virBufferAddLit(&buf, "</domain>\n"); if (virBufferError(&buf)) { xenXMError(conn, VIR_ERR_NO_MEMORY, _("allocate buffer")); - return NULL; + goto error; } return virBufferContentAndReset(&buf); + + error: + free(virBufferContentAndReset(&buf)); + return NULL; } @@ -2311,6 +2329,17 @@ virConfPtr xenXMParseXMLToConfig(virConnectPtr conn, const char *xml) { goto error; } } + + if (virXPathNode("/domain/devices/sound", ctxt)) { + char *soundstr; + if (!(soundstr = virBuildSoundStringFromXML(conn, ctxt))) + goto error; + if (xenXMConfigSetString(conf, "soundhw", soundstr) < 0) { + free(soundstr); + goto error; + } + free(soundstr); + } } xmlFreeDoc(doc); diff --git a/src/xml.c b/src/xml.c index 5381cb1..e9e2e64 100644 --- a/src/xml.c +++ b/src/xml.c @@ -29,6 +29,7 @@ #include "util.h" #include "xs_internal.h" /* for xenStoreDomainGetNetworkID */ #include "xen_unified.h" +#include "xend_internal.h" /* for is_sound_* functions */ /** * virXMLError: @@ -288,6 +289,78 @@ virConvertCpuSet(virConnectPtr conn, const char *str, int maxcpu) { free(cpuset); return (res); } + +/** + * virBuildSoundStringFromXML + * @sound buffer to populate + * @len size of preallocated buffer 'sound' + * @ctxt xml context to pull sound info from + * + * Builds a string of the form m1,m2,m3 from the different sound models + * in the xml. String must be free'd by caller. + * + * Returns string on success, NULL on error + */ +char * virBuildSoundStringFromXML(virConnectPtr conn, + xmlXPathContextPtr ctxt) { + + int nb_nodes, size = 256; + char *sound; + xmlNodePtr *nodes = NULL; + + if (!(sound = calloc(1, size+1))) { + virXMLError(conn, VIR_ERR_NO_MEMORY, + _("failed to allocate sound string"), 0); + return NULL; + } + + nb_nodes = virXPathNodeSet("/domain/devices/sound", ctxt, &nodes); + if (nb_nodes > 0) { + int i; + for (i = 0; i < nb_nodes && size > 0; i++) { + char *model = NULL; + int collision = 0; + + model = (char *) xmlGetProp(nodes[i], (xmlChar *) "model"); + if (!model) { + virXMLError(conn, VIR_ERR_XML_ERROR, + _("no model for sound device"), 0); + goto error; + } + + if (!is_sound_model_valid(model)) { + virXMLError(conn, VIR_ERR_XML_ERROR, + _("unknown sound model type"), 0); + free(model); + goto error; + } + + // Check for duplicates in currently built string + if (*sound) + collision = is_sound_model_conflict(model, sound); + + // If no collision, add to string + if (!collision) { + if (*sound && (size >= (strlen(model) + 1))) { + strncat(sound, ",", size--); + } else if (*sound || size < strlen(model)) { + free(model); + continue; + } + strncat(sound, model, size); + size -= strlen(model); + } + + free(model); + } + } + free(nodes); + return sound; + + error: + free(nodes); + return NULL; +} #endif /* WITH_XEN */ #ifndef PROXY @@ -969,7 +1042,6 @@ virDomainParseXMLOSDescHVM(virConnectPtr conn, xmlNodePtr node, } } - /* get the cdrom device file */ /* Only XenD <= 3.0.2 wants cdrom config here */ if (xendConfigVersion == 1) { @@ -1077,6 +1149,15 @@ virDomainParseXMLOSDescHVM(virConnectPtr conn, xmlNodePtr node, } } + cur = virXPathNode("/domain/devices/sound", ctxt); + if (cur) { + char *soundstr; + if (!(soundstr = virBuildSoundStringFromXML(conn, ctxt))) + goto error; + virBufferVSprintf(buf, "(soundhw '%s')", soundstr); + free(soundstr); + } + str = virXPathString("string(/domain/clock/@offset)", ctxt); if (str != NULL && STREQ(str, "localtime")) { virBufferAddLit(buf, "(localtime 1)"); diff --git a/src/xml.h b/src/xml.h index d91a1b0..e5f2103 100644 --- a/src/xml.h +++ b/src/xml.h @@ -61,6 +61,8 @@ int virDomainXMLDevID(virDomainPtr domain, char *class, char *ref, int ref_len); +char * virBuildSoundStringFromXML(virConnectPtr conn, + xmlXPathContextPtr ctxt); #endif #ifdef __cplusplus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound.args b/tests/qemuxml2argvdata/qemuxml2argv-sound.args new file mode 100644 index 0000000..b8d7533 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound.args @@ -0,0 +1 @@ +/usr/bin/qemu -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -soundhw pcspk,es1370,sb16 \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound.xml new file mode 100644 index 0000000..a351f8d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda'/> + </disk> + <sound model='pcspk'/> + <sound model='pcspk'/> + <sound model='es1370'/> + <sound model='pcspk'/> + <sound model='sb16'/> + <sound model='es1370'/> + <sound model='pcspk'/> + <sound model='sb16'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e7f6021..63abebe 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -159,6 +159,7 @@ main(int argc, char **argv) DO_TEST("serial-many"); DO_TEST("parallel-tcp"); DO_TEST("console-compat"); + DO_TEST("sound"); virCapabilitiesFree(driver.caps); diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.sexpr b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.sexpr new file mode 100644 index 0000000..415fb7b --- /dev/null +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.sexpr @@ -0,0 +1 @@ +(domain (domid 3)(name 'fvtest')(memory 400)(maxmem 400)(vcpus 1)(uuid 'b5d70dd275cdaca517769660b059d8bc')(on_poweroff 'destroy')(on_reboot 'restart')(on_crash 'restart')(image (hvm (kernel '/usr/lib/xen/boot/hvmloader')(device_model '/usr/lib64/xen/bin/qemu-dm')(boot c)(cdrom '/root/boot.iso')(acpi 1)(vnc 1)(keymap ja)(soundhw 'idontexit,es1370,all')))(device (vbd (dev 'ioemu:hda')(uname 'file:/root/foo.img')(mode 'w')))(device (vif (mac '00:16:3e:1b:b1:47')(bridge 'xenbr0')(script 'vif-bridge')(type ioemu)))) diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml new file mode 100644 index 0000000..e316a85 --- /dev/null +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml @@ -0,0 +1,42 @@ +<domain type='xen' id='3'> + <name>fvtest</name> + <uuid>b5d70dd2-75cd-aca5-1776-9660b059d8bc</uuid> + <os> + <type>hvm</type> + <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='hd'/> + </os> + <memory>409600</memory> + <vcpu>1</vcpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <devices> + <emulator>/usr/lib64/xen/bin/qemu-dm</emulator> + <disk type='file' device='disk'> + <driver name='file'/> + <source file='/root/foo.img'/> + <target dev='hda'/> + </disk> + <interface type='bridge'> + <source bridge='xenbr0'/> + <target dev='vif3.0'/> + <mac address='00:16:3e:1b:b1:47'/> + <script path='vif-bridge'/> + </interface> + <disk type='file' device='cdrom'> + <driver name='file'/> + <source file='/root/boot.iso'/> + <target dev='hdc'/> + <readonly/> + </disk> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='5903' keymap='ja'/> + <sound model='sb16'/> + <sound model='es1370'/> + </devices> +</domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound.sexpr b/tests/sexpr2xmldata/sexpr2xml-fv-sound.sexpr new file mode 100644 index 0000000..ab21c09 --- /dev/null +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound.sexpr @@ -0,0 +1 @@ +(domain (domid 3)(name 'fvtest')(memory 400)(maxmem 400)(vcpus 1)(uuid 'b5d70dd275cdaca517769660b059d8bc')(on_poweroff 'destroy')(on_reboot 'restart')(on_crash 'restart')(image (hvm (kernel '/usr/lib/xen/boot/hvmloader')(device_model '/usr/lib64/xen/bin/qemu-dm')(boot c)(cdrom '/root/boot.iso')(acpi 1)(vnc 1)(keymap ja)(soundhw 'sb16,es1370,idontexist,es1370more')))(device (vbd (dev 'ioemu:hda')(uname 'file:/root/foo.img')(mode 'w')))(device (vif (mac '00:16:3e:1b:b1:47')(bridge 'xenbr0')(script 'vif-bridge')(type ioemu)))) diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml b/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml new file mode 100644 index 0000000..e316a85 --- /dev/null +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml @@ -0,0 +1,42 @@ +<domain type='xen' id='3'> + <name>fvtest</name> + <uuid>b5d70dd2-75cd-aca5-1776-9660b059d8bc</uuid> + <os> + <type>hvm</type> + <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='hd'/> + </os> + <memory>409600</memory> + <vcpu>1</vcpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <devices> + <emulator>/usr/lib64/xen/bin/qemu-dm</emulator> + <disk type='file' device='disk'> + <driver name='file'/> + <source file='/root/foo.img'/> + <target dev='hda'/> + </disk> + <interface type='bridge'> + <source bridge='xenbr0'/> + <target dev='vif3.0'/> + <mac address='00:16:3e:1b:b1:47'/> + <script path='vif-bridge'/> + </interface> + <disk type='file' device='cdrom'> + <driver name='file'/> + <source file='/root/boot.iso'/> + <target dev='hdc'/> + <readonly/> + </disk> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='5903' keymap='ja'/> + <sound model='sb16'/> + <sound model='es1370'/> + </devices> +</domain> diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c index b7103d6..6a27b40 100644 --- a/tests/sexpr2xmltest.c +++ b/tests/sexpr2xmltest.c @@ -136,6 +136,9 @@ main(int argc, char **argv) DO_TEST("fv-serial-unix", "fv-serial-unix", 1); DO_TEST("fv-parallel-tcp", "fv-parallel-tcp", 1); + DO_TEST("fv-sound", "fv-sound", 1); + DO_TEST("fv-sound-all", "fv-sound-all", 1); + exit(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } #else /* WITHOUT_XEN */ diff --git a/tests/xmconfigdata/test-fullvirt-sound.cfg b/tests/xmconfigdata/test-fullvirt-sound.cfg new file mode 100755 index 0000000..7d77f94 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-sound.cfg @@ -0,0 +1,26 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +builder = "hvm" +kernel = "/usr/lib/xen/boot/hvmloader" +boot = "d" +pae = 1 +acpi = 1 +apic = 1 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-dm" +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = "127.0.0.1" +vncpasswd = "123poi" +disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso,hdc:cdrom,r" ] +vif = [ "mac=00:16:3E:66:92:9C,bridge=xenbr1,type=ioemu" ] +parallel = "none" +serial = "none" +soundhw = "sb16,es1370" diff --git a/tests/xmconfigdata/test-fullvirt-sound.xml b/tests/xmconfigdata/test-fullvirt-sound.xml new file mode 100644 index 0000000..5ecf955 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-sound.xml @@ -0,0 +1,43 @@ +<domain type='xen'> + <name>XenGuest2</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <os> + <type>hvm</type> + <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <currentMemory>403456</currentMemory> + <memory>592896</memory> + <vcpu>1</vcpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <features> + <pae/> + <acpi/> + <apic/> + </features> + <clock offset='utc'/> + <devices> + <emulator>/usr/lib/xen/bin/qemu-dm</emulator> + <disk type='block' device='disk'> + <driver name='phy'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='file'/> + <source file='/root/boot.iso'/> + <target dev='hdc'/> + <readonly/> + </disk> + <interface type='bridge'> + <mac address='00:16:3E:66:92:9C'/> + <source bridge='xenbr1'/> + </interface> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' listen='127.0.0.1' passwd='123poi'/> + <sound model='sb16'/> + <sound model='es1370'/> + </devices> +</domain> diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index de446aa..3725ae5 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -221,6 +221,8 @@ main(int argc, char **argv) DO_TEST("fullvirt-parallel-tcp", 2); + DO_TEST("fullvirt-sound", 2); + exit(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } #else /* WITHOUT_XEN */ diff --git a/tests/xml2sexprdata/xml2sexpr-fv-sound.sexpr b/tests/xml2sexprdata/xml2sexpr-fv-sound.sexpr new file mode 100644 index 0000000..71f0a62 --- /dev/null +++ b/tests/xml2sexprdata/xml2sexpr-fv-sound.sexpr @@ -0,0 +1 @@ +(vm (name 'fvtest')(memory 400)(maxmem 400)(vcpus 1)(uuid 'b5d70dd275cdaca517769660b059d8bc')(on_poweroff 'destroy')(on_reboot 'restart')(on_crash 'restart')(image (hvm (kernel '/usr/lib/xen/boot/hvmloader')(vcpus 1)(boot c)(cdrom '/root/boot.iso')(acpi 1)(usb 1)(parallel none)(serial none)(soundhw 'sb16,es1370')(device_model '/usr/lib64/xen/bin/qemu-dm')(vnc 1)))(device (vbd (dev 'ioemu:hda')(uname 'file:/root/foo.img')(mode 'w')))(device (vif (mac '00:16:3e:1b:b1:47')(bridge 'xenbr0')(script 'vif-bridge')(type ioemu)))) \ No newline at end of file diff --git a/tests/xml2sexprdata/xml2sexpr-fv-sound.xml b/tests/xml2sexprdata/xml2sexpr-fv-sound.xml new file mode 100644 index 0000000..863659c --- /dev/null +++ b/tests/xml2sexprdata/xml2sexpr-fv-sound.xml @@ -0,0 +1,41 @@ +<domain type='xen'> + <name>fvtest</name> + <uuid>b5d70dd275cdaca517769660b059d8bc</uuid> + <os> + <type>hvm</type> + <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='hd'/> + </os> + <memory>409600</memory> + <vcpu>1</vcpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <features> + <acpi/> + </features> + <devices> + <emulator>/usr/lib64/xen/bin/qemu-dm</emulator> + <interface type='bridge'> + <source bridge='xenbr0'/> + <mac address='00:16:3e:1b:b1:47'/> + <script path='vif-bridge'/> + </interface> + <disk type='file' device='cdrom'> + <source file='/root/boot.iso'/> + <target dev='hdc'/> + <readonly/> + </disk> + <disk type='file'> + <source file='/root/foo.img'/> + <target dev='ioemu:hda'/> + </disk> + <graphics type='vnc' port='5917' keymap='ja'/> + <sound model='sb16'/> + <sound model='sb16'/> + <sound model='es1370'/> + <sound model='sb16'/> + <sound model='es1370'/> + </devices> +</domain> + diff --git a/tests/xml2sexprtest.c b/tests/xml2sexprtest.c index d04233a..547c66f 100644 --- a/tests/xml2sexprtest.c +++ b/tests/xml2sexprtest.c @@ -143,6 +143,8 @@ main(int argc, char **argv) DO_TEST("fv-serial-unix", "fv-serial-unix", "fvtest", 1); DO_TEST("fv-parallel-tcp", "fv-parallel-tcp", "fvtest", 1); + DO_TEST("fv-sound", "fv-sound", "fvtest", 1); + exit(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); }
On Tue, May 06, 2008 at 10:39:53AM -0400, Cole Robinson wrote:
Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen.
Third iteration. Changes include:
- Rediffd against current code - Reading 'all' from a xend sexpr or xm config does the right thing. - Added test files.
Great, this looks good now
- Addressed most of Jim's feedback. Unfortunately I could not reuse the qemu sound functions, as xen (on f8 at least) doesn't support the pcspk model, so the whitelist isn't the same between the two.
I don't think its a huge deal to have separate code for Xen vs QEMU. The entire XML parser needs to be merged - just dealing with the sound functions is such a minor deal its not worth it. Regards, Daniel. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, May 06, 2008 at 10:25:37PM +0100, Daniel P. Berrange wrote:
On Tue, May 06, 2008 at 10:39:53AM -0400, Cole Robinson wrote:
Cole Robinson wrote:
The patch below adds xml support for the soundhw option to qemu and xen.
Third iteration. Changes include:
- Rediffd against current code - Reading 'all' from a xend sexpr or xm config does the right thing. - Added test files.
Great, this looks good now
Excellent, this is now commited. One thing left to do is document the associated XML extentions for the devices sections. Should be easier now with Dan's new HTML generation, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Hi, Cole and Dan I have a question about the reason why it does not support in xen_proxy. Would you explain why it does not support in xen_proxy? This policy comes from security issue or other? If it does already commented on this please show me a pointer. P.S. I raise this question when I see the patch of follows. But I cannot find the reason. Disable sound functions when in proxy http://git.et.redhat.com/?p=libvirt.git;a=commit;h=0f7619f25e7dd03296c95241c... Thanks Atsushi SAKAI Cole Robinson <crobinso@redhat.com> wrote:
The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form:
<sound driver='drivername'/>
Where driver name can be pcspk, sb16, es1370, or all.
Everything seems to be in working order but I have a few implementation questions:
1) Should multiple drivers be able to be specified? qemu accommodates this, allowing '-soundhw sb16,pcspk' for example. If this should be allowed, what should the xml format be?
2) Should acceptable driver options be hardcoded? The other option is to just pass the input straight to qemu. This patch has the options hardcoded.
Also I realize this will probably need to be rediff'd around Dan's serial + parallel device patch, but I figured I would just get this out there.
Thanks, Cole
On Fri, May 09, 2008 at 06:33:19PM +0900, Atsushi SAKAI wrote:
Hi, Cole and Dan
I have a question about the reason why it does not support in xen_proxy. Would you explain why it does not support in xen_proxy?
That changeset was just disabling a couple of functions there were not needed in the proxy. The proxy is read-only, so doesn't need ability to create VMs & thus those sound functions were disabled. Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (6)
-
Atsushi SAKAI -
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard -
Jerone Young -
Jim Meyering