Re: [Libvir] [PATCH] sound support for qemu and xen

Cole Robinson wrote: 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