[libvirt] [PATCH libvirt 1/8] qemu: test CAPS_HDA_MICRO

--- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemuhelptest.c | 3 ++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a3c87d1..b410648 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -162,6 +162,9 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "scsi-cd", "ide-cd", "no-user-config", + + "hda-micro", /* 95 */ + ); struct qemu_feature_flags { @@ -1397,6 +1400,8 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags) /* Which devices exist. */ if (strstr(str, "name \"hda-duplex\"")) qemuCapsSet(flags, QEMU_CAPS_HDA_DUPLEX); + if (strstr(str, "name \"hda-micro\"")) + qemuCapsSet(flags, QEMU_CAPS_HDA_MICRO); if (strstr(str, "name \"ccid-card-emulated\"")) qemuCapsSet(flags, QEMU_CAPS_CCID_EMULATED); if (strstr(str, "name \"ccid-card-passthru\"")) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0e0899e..64831e2 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -130,6 +130,7 @@ enum qemuCapsFlags { QEMU_CAPS_SCSI_CD = 92, /* -device scsi-cd */ QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */ QEMU_CAPS_NO_USER_CONFIG = 94, /* -no-user-config */ + QEMU_CAPS_HDA_MICRO = 95, /* -device hda-micro */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 57d1859..46cc973 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -752,7 +752,8 @@ mymain(void) QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_SCSI_CD, QEMU_CAPS_IDE_CD, - QEMU_CAPS_NO_USER_CONFIG); + QEMU_CAPS_NO_USER_CONFIG, + QEMU_CAPS_HDA_MICRO); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.7.10.1

Allow specifying sound device codecs. See formatdomain.html for more details. --- docs/formatdomain.html.in | 19 +++++++ docs/schemas/domaincommon.rng | 29 +++++++--- src/conf/domain_conf.c | 120 ++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 20 +++++++ 4 files changed, 180 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index da61312..3875167 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3617,6 +3617,25 @@ qemu-kvm -net nic,model=? /dev/null </dl> <p> + <span class="since">Since 0.9.13</span>, a sound element + with <code>ich6</code> model can have optional + sub-elements <code><codec></code> to attach various audio + codecs to the audio device. If not specified, a default codec + will be attached to allow playback and recording. Valid values + are 'duplex' (advertise a line-in and a line-out) and 'micro' + (advertise a speaker and a microphone). + </p> + +<pre> + ... + <devices> + <sound model='ich6'> + <codec type='micro'/> + <sound/> + </devices> + ...</pre> + + <p> Each <code>sound</code> element has an optional sub-element <code><address></code> which can tie the device to a particular PCI diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4c4fa6a..34f63c3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2250,6 +2250,16 @@ </choice> </element> </define> + <define name="codec"> + <element name="codec"> + <attribute name="type"> + <choice> + <value>duplex</value> + <value>micro</value> + </choice> + </attribute> + </element> + </define> <define name="sound"> <element name="sound"> <attribute name="model"> @@ -2261,12 +2271,19 @@ <value>ich6</value> </choice> </attribute> - <optional> - <ref name="alias"/> - </optional> - <optional> - <ref name="address"/> - </optional> + <interleave> + <optional> + <ref name="alias"/> + </optional> + <optional> + <ref name="address"/> + </optional> + <zeroOrMore> + <choice> + <ref name="codec"/> + </choice> + </zeroOrMore> + </interleave> </element> </define> <define name="watchdog"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 79355e2..78755cf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -354,6 +354,10 @@ VIR_ENUM_IMPL(virDomainSmartcard, VIR_DOMAIN_SMARTCARD_TYPE_LAST, "host-certificates", "passthrough") +VIR_ENUM_IMPL(virDomainSoundCodec, VIR_DOMAIN_SOUND_CODEC_TYPE_LAST, + "duplex", + "micro") + VIR_ENUM_IMPL(virDomainSoundModel, VIR_DOMAIN_SOUND_MODEL_LAST, "sb16", "es1370", @@ -1304,6 +1308,14 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def) VIR_FREE(def); } +void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def); +} + void virDomainSoundDefFree(virDomainSoundDefPtr def) { if (!def) @@ -1311,6 +1323,11 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def) virDomainDeviceInfoClear(&def->info); + int i; + for (i = 0 ; i < def->ncodecs ; i++) + virDomainSoundCodecDefFree(def->codecs[i]); + VIR_FREE(def->codecs); + VIR_FREE(def); } @@ -6374,18 +6391,52 @@ error: } +static virDomainSoundCodecDefPtr +virDomainSoundCodecDefParseXML(const xmlNodePtr node) +{ + char *type; + virDomainSoundCodecDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + type = virXMLPropString(node, "type"); + if ((def->type = virDomainSoundCodecTypeFromString(type)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown codec type '%s'"), type); + goto error; + } + +cleanup: + VIR_FREE(type); + + return def; + +error: + virDomainSoundCodecDefFree(def); + def = NULL; + goto cleanup; +} + + static virDomainSoundDefPtr virDomainSoundDefParseXML(const xmlNodePtr node, + xmlXPathContextPtr ctxt, unsigned int flags) { char *model; virDomainSoundDefPtr def; + xmlNodePtr save = ctxt->node; if (VIR_ALLOC(def) < 0) { virReportOOMError(); return NULL; } + ctxt->node = node; + model = virXMLPropString(node, "model"); if ((def->model = virDomainSoundModelTypeFromString(model)) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -6393,12 +6444,43 @@ virDomainSoundDefParseXML(const xmlNodePtr node, goto error; } + if (def->model == VIR_DOMAIN_SOUND_MODEL_ICH6) { + int ncodecs; + xmlNodePtr *codecNodes = NULL; + + /* parse the <codec> subelements for sound models that support it */ + ncodecs = virXPathNodeSet("./codec", ctxt, &codecNodes); + if (ncodecs < 0) + goto error; + + if (ncodecs > 0) { + int ii; + + if (VIR_ALLOC_N(def->codecs, ncodecs) < 0) { + virReportOOMError(); + VIR_FREE(codecNodes); + goto error; + } + + for (ii = 0; ii < ncodecs; ii++) { + virDomainSoundCodecDefPtr codec = virDomainSoundCodecDefParseXML(codecNodes[ii]); + if (codec == NULL) + goto error; + + codec->cad = def->ncodecs; /* that will do for now */ + def->codecs[def->ncodecs++] = codec; + } + VIR_FREE(codecNodes); + } + } + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; cleanup: VIR_FREE(model); + ctxt->node = save; return def; error: @@ -6951,7 +7033,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } else if (xmlStrEqual(node->name, BAD_CAST "sound")) { dev->type = VIR_DOMAIN_DEVICE_SOUND; - if (!(dev->data.sound = virDomainSoundDefParseXML(node, flags))) + if (!(dev->data.sound = virDomainSoundDefParseXML(node, ctxt, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "watchdog")) { dev->type = VIR_DOMAIN_DEVICE_WATCHDOG; @@ -8818,6 +8900,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { virDomainSoundDefPtr sound = virDomainSoundDefParseXML(nodes[i], + ctxt, flags); if (!sound) goto error; @@ -11783,11 +11866,30 @@ virDomainSmartcardDefFormat(virBufferPtr buf, } static int +virDomainSoundCodecDefFormat(virBufferPtr buf, + virDomainSoundCodecDefPtr def) +{ + const char *type = virDomainSoundCodecTypeToString(def->type); + + if (!type) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected codec type %d"), def->type); + return -1; + } + + virBufferAsprintf(buf, " <codec type='%s'/>\n", type); + + return 0; +} + +static int virDomainSoundDefFormat(virBufferPtr buf, virDomainSoundDefPtr def, unsigned int flags) { const char *model = virDomainSoundModelTypeToString(def->model); + int children = 0; + int i; if (!model) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -11797,10 +11899,24 @@ virDomainSoundDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <sound model='%s'", model); + for (i = 0; i < def->ncodecs; i++) { + if (!children) { + virBufferAddLit(buf, ">\n"); + children = 1; + } + virDomainSoundCodecDefFormat(buf, def->codecs[i]); + } + if (virDomainDeviceInfoIsSet(&def->info, flags)) { - virBufferAddLit(buf, ">\n"); + if (!children) { + virBufferAddLit(buf, ">\n"); + children = 1; + } if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; + } + + if (children) { virBufferAddLit(buf, " </sound>\n"); } else { virBufferAddLit(buf, "/>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1597212..4c56902 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -65,6 +65,9 @@ typedef virDomainNetDef *virDomainNetDefPtr; typedef struct _virDomainInputDef virDomainInputDef; typedef virDomainInputDef *virDomainInputDefPtr; +typedef struct _virDomainSoundCodecDef virDomainSoundCodecDef; +typedef virDomainSoundCodecDef *virDomainSoundCodecDefPtr; + typedef struct _virDomainSoundDef virDomainSoundDef; typedef virDomainSoundDef *virDomainSoundDefPtr; @@ -996,6 +999,13 @@ struct _virDomainInputDef { virDomainDeviceInfo info; }; +enum virDomainSoundCodecType { + VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX, + VIR_DOMAIN_SOUND_CODEC_TYPE_MICRO, + + VIR_DOMAIN_SOUND_CODEC_TYPE_LAST +}; + enum virDomainSoundModel { VIR_DOMAIN_SOUND_MODEL_SB16, VIR_DOMAIN_SOUND_MODEL_ES1370, @@ -1006,9 +1016,17 @@ enum virDomainSoundModel { VIR_DOMAIN_SOUND_MODEL_LAST }; +struct _virDomainSoundCodecDef { + int type; + int cad; +}; + struct _virDomainSoundDef { int model; virDomainDeviceInfo info; + + int ncodecs; + virDomainSoundCodecDefPtr *codecs; }; enum virDomainWatchdogModel { @@ -1848,6 +1866,7 @@ void virDomainChrDefFree(virDomainChrDefPtr def); void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def); int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr src, virDomainChrSourceDefPtr dest); +void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def); void virDomainSoundDefFree(virDomainSoundDefPtr def); void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); @@ -2164,6 +2183,7 @@ VIR_ENUM_DECL(virDomainSmartcard) VIR_ENUM_DECL(virDomainChr) VIR_ENUM_DECL(virDomainChrTcpProtocol) VIR_ENUM_DECL(virDomainChrSpicevmc) +VIR_ENUM_DECL(virDomainSoundCodec) VIR_ENUM_DECL(virDomainSoundModel) VIR_ENUM_DECL(virDomainMemballoonModel) VIR_ENUM_DECL(virDomainSmbiosMode) -- 1.7.10.1

On 05/15/2012 04:55 PM, Marc-André Lureau wrote:
Allow specifying sound device codecs. See formatdomain.html for more details. --- docs/formatdomain.html.in | 19 +++++++ docs/schemas/domaincommon.rng | 29 +++++++--- src/conf/domain_conf.c | 120 ++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 20 +++++++ 4 files changed, 180 insertions(+), 8 deletions(-)
+static int virDomainSoundDefFormat(virBufferPtr buf, virDomainSoundDefPtr def, unsigned int flags) { const char *model = virDomainSoundModelTypeToString(def->model); + int children = 0;
You are using this as a bool. ACK with this squashed in: diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index 78755cf..9def71d 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -11888,7 +11888,7 @@ virDomainSoundDefFormat(virBufferPtr buf, unsigned int flags) { const char *model = virDomainSoundModelTypeToString(def->model); - int children = 0; + bool children = false; int i; if (!model) { @@ -11902,7 +11902,7 @@ virDomainSoundDefFormat(virBufferPtr buf, for (i = 0; i < def->ncodecs; i++) { if (!children) { virBufferAddLit(buf, ">\n"); - children = 1; + children = true; } virDomainSoundCodecDefFormat(buf, def->codecs[i]); } @@ -11910,7 +11910,7 @@ virDomainSoundDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { if (!children) { virBufferAddLit(buf, ">\n"); - children = 1; + children = true; } if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

With ICH6 audio device, allow to specify codecs. By default, for compatibility reasons, if no codec is specified, "hda-duplex" will be used. --- src/qemu/qemu_command.c | 74 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3a92c70..9f99dce 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -89,6 +89,12 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "", /* don't support vbox */ "qxl"); +VIR_ENUM_DECL(qemuSoundCodec) + +VIR_ENUM_IMPL(qemuSoundCodec, VIR_DOMAIN_SOUND_CODEC_TYPE_LAST, + "hda-duplex", + "hda-micro"); + VIR_ENUM_DECL(qemuControllerModelUSB) VIR_ENUM_IMPL(qemuControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, @@ -3038,21 +3044,43 @@ error: return NULL; } + +static int +qemuSoundCodecTypeToCaps(int type) +{ + switch (type) { + case VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX: + return QEMU_CAPS_HDA_DUPLEX; + case VIR_DOMAIN_SOUND_CODEC_TYPE_MICRO: + return QEMU_CAPS_HDA_MICRO; + default: + return -1; + } +} + + static char * qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, - const char *codec) + virDomainSoundCodecDefPtr codec, + virBitmapPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; - int cad = 0; + const char *stype; + int type, caps; - virBufferAsprintf(&buf, "%s,id=%s-codec%d,bus=%s.0,cad=%d", - codec, sound->info.alias, cad, sound->info.alias, cad); + type = codec->type; + stype = qemuSoundCodecTypeToString(type); + caps = qemuSoundCodecTypeToCaps(type); - if (virBufferError(&buf)) { - virReportOOMError(); + if (caps == -1 || !qemuCapsGet(qemuCaps, caps)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("%s not supported in this QEMU binary"), stype); goto error; } + virBufferAsprintf(&buf, "%s,id=%s-codec%d,bus=%s.0,cad=%d", + stype, sound->info.alias, codec->cad, sound->info.alias, codec->cad); + return virBufferContentAndReset(&buf); error: @@ -5815,20 +5843,30 @@ qemuBuildCommandLine(virConnectPtr conn, if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6) { char *codecstr = NULL; - if (!qemuCapsGet(qemuCaps, QEMU_CAPS_HDA_DUPLEX)) { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks hda support")); - goto error; - } + int ii; - virCommandAddArg(cmd, "-device"); - if (!(codecstr = qemuBuildSoundCodecStr(sound, - "hda-duplex"))) { - goto error; - } + for (ii = 0 ; ii < sound->ncodecs ; ii++) { + virCommandAddArg(cmd, "-device"); + if (!(codecstr = qemuBuildSoundCodecStr(sound, sound->codecs[ii], qemuCaps))) { + goto error; - virCommandAddArg(cmd, codecstr); - VIR_FREE(codecstr); + } + virCommandAddArg(cmd, codecstr); + VIR_FREE(codecstr); + } + if (ii == 0) { + virDomainSoundCodecDef codec = { + VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX, + 0 + }; + virCommandAddArg(cmd, "-device"); + if (!(codecstr = qemuBuildSoundCodecStr(sound, &codec, qemuCaps))) { + goto error; + + } + virCommandAddArg(cmd, codecstr); + VIR_FREE(codecstr); + } } VIR_FREE(str); -- 1.7.10.1

On 05/15/2012 04:55 PM, Marc-André Lureau wrote:
With ICH6 audio device, allow to specify codecs. By default, for compatibility reasons, if no codec is specified, "hda-duplex" will be used. --- src/qemu/qemu_command.c | 74 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 18 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Test new codec type element. --- tests/qemuxml2argvdata/qemuxml2argv-sound-device.args | 7 +++++-- tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml | 7 +++++++ tests/qemuxml2argvtest.c | 3 ++- tests/qemuxml2xmltest.c | 1 + 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args index 0a09c41..4a7129b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args @@ -4,5 +4,8 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -usb -soundhw pcspk -device ES1370,id=sound1,bus=pci.0,\ addr=0x3 -device sb16,id=sound2 -device AC97,id=sound3,bus=pci.0,addr=0x4 \ -device intel-hda,id=sound4,bus=pci.0,addr=0x5 -device hda-duplex,\ -id=sound4-codec0,bus=sound4.0,cad=0 -device virtio-balloon-pci,id=balloon0,\ -bus=pci.0,addr=0x6 +id=sound4-codec0,bus=sound4.0,cad=0 \ +-device intel-hda,id=sound5,bus=pci.0,addr=0x6 \ +-device hda-micro,id=sound5-codec0,bus=sound5.0,cad=0 \ +-device hda-duplex,id=sound5-codec1,bus=sound5.0,cad=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml index 33ec569..c588a24 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml @@ -17,12 +17,19 @@ <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> <sound model='pcspk'/> <sound model='es1370'/> <sound model='sb16'/> <sound model='ac97'/> <sound model='ich6'/> + <sound model='ich6'> + <codec type='micro'/> + <codec type='duplex'/> + </sound> <memballoon model='virtio'/> </devices> </domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c073429..617b178 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -680,7 +680,8 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("sound", false, NONE); DO_TEST("sound-device", false, - QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_HDA_DUPLEX); + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_HDA_DUPLEX, QEMU_CAPS_HDA_MICRO); DO_TEST("fs9p", false, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_FSDEV, QEMU_CAPS_FSDEV_WRITEOUT); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index de76064..dcdba4f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -179,6 +179,7 @@ mymain(void) DO_TEST("net-virtio-network-portgroup"); DO_TEST("net-hostdev"); DO_TEST("sound"); + DO_TEST("sound-device"); DO_TEST("net-bandwidth"); DO_TEST("serial-vc"); -- 1.7.10.1

On 05/15/2012 04:55 PM, Marc-André Lureau wrote:
Test new codec type element. --- tests/qemuxml2argvdata/qemuxml2argv-sound-device.args | 7 +++++-- tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml | 7 +++++++ tests/qemuxml2argvtest.c | 3 ++- tests/qemuxml2xmltest.c | 1 + 4 files changed, 15 insertions(+), 3 deletions(-)
ACK. I've pushed 1-4. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Allow to specify preallocation mode for QCOW2 images. If not specified or not available, it's ignored. This change only modify the schema, doc, parsing and tests. --- docs/formatstorage.html.in | 6 ++++++ docs/schemas/storagevol.rng | 18 ++++++++++++++++++ src/conf/storage_conf.c | 26 ++++++++++++++++++++++++++ src/conf/storage_conf.h | 1 + src/util/storage_file.c | 4 ++++ src/util/storage_file.h | 10 ++++++++++ tests/storagevolxml2xmlin/vol-qcow2.xml | 1 + tests/storagevolxml2xmlout/vol-qcow2.xml | 1 + 8 files changed, 67 insertions(+) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index d0e4319..8fd2908 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -252,6 +252,12 @@ 1,152,921,504,606,846,976 bytes). <span class="since">Since 0.4.1, multi-character <code>unit</code> since 0.9.11</span></dd> + <dt><code>preallocation</code></dt> + <dd>An image with preallocated metadata is initially larger but + can improve performance when the image needs to grow. This is + supported by the QCOW2 image format. + Attribute <code>mode</code> value can be 'off' or 'metadata'. + <span class="since">Since 0.9.13</span></dd> <dt><code>capacity</code></dt> <dd>Providing the logical capacity for the volume. This value is in bytes by default, but a <code>unit</code> attribute can be diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 8edb877..b54db13 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -40,6 +40,9 @@ <ref name='scaledInteger'/> </element> </optional> + <optional> + <ref name='preallocation'/> + </optional> </define> <define name='permissions'> @@ -171,6 +174,21 @@ </optional> </define> + <define name='preallocationmode'> + <choice> + <value>off</value> + <value>metadata</value> + </choice> + </define> + + <define name='preallocation'> + <element name='preallocation'> + <attribute name='mode'> + <ref name='preallocationmode'/> + </attribute> + </element> + </define> + <define name='name'> <data type='string'> <param name="pattern">[a-zA-Z0-9_\+\-\.]+</param> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 188af6d..b775ad2 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -992,6 +992,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, char *allocation = NULL; char *capacity = NULL; char *unit = NULL; + char *preallocation = NULL; xmlNodePtr node; options = virStorageVolOptionsForPoolType(pool->type); @@ -1036,6 +1037,18 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, ret->allocation = ret->capacity; } + preallocation = virXPathString("string(./preallocation/@mode)", ctxt); + if (preallocation) { + if ((ret->preallocation = virStoragePreallocationModeTypeFromString(preallocation)) < 0) { + virStorageReportError(VIR_ERR_XML_ERROR, + _("unknown preallocation mode %s"), preallocation); + goto cleanup; + } + VIR_FREE(preallocation); + } else { + ret->preallocation = VIR_STORAGE_PREALLOCATION_NONE; + } + ret->target.path = virXPathString("string(./target/path)", ctxt); if (options->formatFromString) { char *format = virXPathString("string(./target/format/@type)", ctxt); @@ -1091,6 +1104,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, return ret; cleanup: + VIR_FREE(preallocation); VIR_FREE(allocation); VIR_FREE(capacity); VIR_FREE(unit); @@ -1245,6 +1259,18 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, virBufferAsprintf(&buf," <allocation unit='bytes'>%llu</allocation>\n", def->allocation); + if (def->preallocation != VIR_STORAGE_PREALLOCATION_NONE) { + const char *preallocation; + + preallocation = virStoragePreallocationModeTypeToString(def->preallocation); + if (!preallocation) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unexpected preallocation mode")); + goto cleanup; + } + virBufferAsprintf(&buf," <preallocation mode='%s'/>\n", preallocation); + } + if (virStorageVolTargetDefFormat(options, &buf, &def->target, "target") < 0) goto cleanup; diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 9222c4a..c7c7af0 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -98,6 +98,7 @@ struct _virStorageVolDef { virStorageVolSource source; virStorageVolTarget target; virStorageVolTarget backingStore; + int preallocation; /* virStoragePreallocationMode enum */ }; typedef struct _virStorageVolDefList virStorageVolDefList; diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 530071e..63abe65 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -47,6 +47,10 @@ VIR_ENUM_IMPL(virStorageFileFormat, "cloop", "cow", "dmg", "iso", "qcow", "qcow2", "qed", "vmdk", "vpc") +VIR_ENUM_IMPL(virStoragePreallocationMode, + VIR_STORAGE_PREALLOCATION_LAST, + "default", "off", "metadata") + enum lv_endian { LV_LITTLE_ENDIAN = 1, /* 1234 */ LV_BIG_ENDIAN /* 4321 */ diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 13d0e87..dfc8719 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -46,6 +46,16 @@ enum virStorageFileFormat { VIR_ENUM_DECL(virStorageFileFormat); +enum virStoragePreallocationMode { + VIR_STORAGE_PREALLOCATION_NONE, + VIR_STORAGE_PREALLOCATION_OFF, + VIR_STORAGE_PREALLOCATION_METADATA, + + VIR_STORAGE_PREALLOCATION_LAST +}; + +VIR_ENUM_DECL(virStoragePreallocationMode); + typedef struct _virStorageFileMetadata { char *backingStore; int backingStoreFormat; diff --git a/tests/storagevolxml2xmlin/vol-qcow2.xml b/tests/storagevolxml2xmlin/vol-qcow2.xml index b4924de..b4c6522 100644 --- a/tests/storagevolxml2xmlin/vol-qcow2.xml +++ b/tests/storagevolxml2xmlin/vol-qcow2.xml @@ -5,6 +5,7 @@ </source> <capacity unit="G">5</capacity> <allocation>294912</allocation> + <preallocation mode='metadata'/> <target> <path>/var/lib/libvirt/images/OtherDemo.img</path> <format type='qcow2'/> diff --git a/tests/storagevolxml2xmlout/vol-qcow2.xml b/tests/storagevolxml2xmlout/vol-qcow2.xml index 4490931..311c52e 100644 --- a/tests/storagevolxml2xmlout/vol-qcow2.xml +++ b/tests/storagevolxml2xmlout/vol-qcow2.xml @@ -5,6 +5,7 @@ </source> <capacity unit='bytes'>5368709120</capacity> <allocation unit='bytes'>294912</allocation> + <preallocation mode='metadata'/> <target> <path>/var/lib/libvirt/images/OtherDemo.img</path> <format type='qcow2'/> -- 1.7.10.1

On Wed, May 16, 2012 at 12:55:12AM +0200, Marc-André Lureau wrote:
Allow to specify preallocation mode for QCOW2 images. If not specified or not available, it's ignored.
This change only modify the schema, doc, parsing and tests. --- docs/formatstorage.html.in | 6 ++++++ docs/schemas/storagevol.rng | 18 ++++++++++++++++++ src/conf/storage_conf.c | 26 ++++++++++++++++++++++++++ src/conf/storage_conf.h | 1 + src/util/storage_file.c | 4 ++++ src/util/storage_file.h | 10 ++++++++++ tests/storagevolxml2xmlin/vol-qcow2.xml | 1 + tests/storagevolxml2xmlout/vol-qcow2.xml | 1 + 8 files changed, 67 insertions(+)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index d0e4319..8fd2908 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -252,6 +252,12 @@ 1,152,921,504,606,846,976 bytes). <span class="since">Since 0.4.1, multi-character <code>unit</code> since 0.9.11</span></dd> + <dt><code>preallocation</code></dt> + <dd>An image with preallocated metadata is initially larger but + can improve performance when the image needs to grow. This is + supported by the QCOW2 image format. + Attribute <code>mode</code> value can be 'off' or 'metadata'. + <span class="since">Since 0.9.13</span></dd>
Sorry for not catching this earlier, but I'd also add a <preallocation mode='metadata'/> line to the <pre> <volume> <name>sparse.img</name> <key>/var/lib/xen/images/sparse.img</key> <allocation>0</allocation> <capacity unit="T">1</capacity> ...</pre> block above.
diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 530071e..63abe65 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -47,6 +47,10 @@ VIR_ENUM_IMPL(virStorageFileFormat, "cloop", "cow", "dmg", "iso", "qcow", "qcow2", "qed", "vmdk", "vpc")
+VIR_ENUM_IMPL(virStoragePreallocationMode, + VIR_STORAGE_PREALLOCATION_LAST, + "default", "off", "metadata") + enum lv_endian { LV_LITTLE_ENDIAN = 1, /* 1234 */ LV_BIG_ENDIAN /* 4321 */ diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 13d0e87..dfc8719 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -46,6 +46,16 @@ enum virStorageFileFormat {
VIR_ENUM_DECL(virStorageFileFormat);
+enum virStoragePreallocationMode { + VIR_STORAGE_PREALLOCATION_NONE,
Nit: I'd call this DEFAULT to go with the string name, but feel free to keep things this way. ACK (though a quick review from someone more familiar with libvirt looking at the XML changes and the error code that are used wouldn't hurt ;) Christophe

Allow to specify preallocation mode for QCOW2 images. If not specified or not available, it's ignored. This change only modify the schema, doc, parsing and tests. --- docs/formatstorage.html.in | 7 +++++++ docs/schemas/storagevol.rng | 18 ++++++++++++++++++ src/conf/storage_conf.c | 26 ++++++++++++++++++++++++++ src/conf/storage_conf.h | 1 + src/util/storage_file.c | 4 ++++ src/util/storage_file.h | 10 ++++++++++ tests/storagevolxml2xmlin/vol-qcow2.xml | 1 + tests/storagevolxml2xmlout/vol-qcow2.xml | 1 + 8 files changed, 68 insertions(+) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index d0e4319..232bd76 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -213,6 +213,7 @@ <key>/var/lib/xen/images/sparse.img</key> <allocation>0</allocation> <capacity unit="T">1</capacity> + <preallocation mode="metadata"/> ...</pre> <dl> @@ -252,6 +253,12 @@ 1,152,921,504,606,846,976 bytes). <span class="since">Since 0.4.1, multi-character <code>unit</code> since 0.9.11</span></dd> + <dt><code>preallocation</code></dt> + <dd>An image with preallocated metadata is initially larger but + can improve performance when the image needs to grow. This is + supported by the QCOW2 image format. + Attribute <code>mode</code> value can be 'off' or 'metadata'. + <span class="since">Since 0.9.13</span></dd> <dt><code>capacity</code></dt> <dd>Providing the logical capacity for the volume. This value is in bytes by default, but a <code>unit</code> attribute can be diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 8edb877..b54db13 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -40,6 +40,9 @@ <ref name='scaledInteger'/> </element> </optional> + <optional> + <ref name='preallocation'/> + </optional> </define> <define name='permissions'> @@ -171,6 +174,21 @@ </optional> </define> + <define name='preallocationmode'> + <choice> + <value>off</value> + <value>metadata</value> + </choice> + </define> + + <define name='preallocation'> + <element name='preallocation'> + <attribute name='mode'> + <ref name='preallocationmode'/> + </attribute> + </element> + </define> + <define name='name'> <data type='string'> <param name="pattern">[a-zA-Z0-9_\+\-\.]+</param> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 188af6d..f0a036b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -992,6 +992,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, char *allocation = NULL; char *capacity = NULL; char *unit = NULL; + char *preallocation = NULL; xmlNodePtr node; options = virStorageVolOptionsForPoolType(pool->type); @@ -1036,6 +1037,18 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, ret->allocation = ret->capacity; } + preallocation = virXPathString("string(./preallocation/@mode)", ctxt); + if (preallocation) { + if ((ret->preallocation = virStoragePreallocationModeTypeFromString(preallocation)) < 0) { + virStorageReportError(VIR_ERR_XML_ERROR, + _("unknown preallocation mode %s"), preallocation); + goto cleanup; + } + VIR_FREE(preallocation); + } else { + ret->preallocation = VIR_STORAGE_PREALLOCATION_DEFAULT; + } + ret->target.path = virXPathString("string(./target/path)", ctxt); if (options->formatFromString) { char *format = virXPathString("string(./target/format/@type)", ctxt); @@ -1091,6 +1104,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, return ret; cleanup: + VIR_FREE(preallocation); VIR_FREE(allocation); VIR_FREE(capacity); VIR_FREE(unit); @@ -1245,6 +1259,18 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, virBufferAsprintf(&buf," <allocation unit='bytes'>%llu</allocation>\n", def->allocation); + if (def->preallocation != VIR_STORAGE_PREALLOCATION_DEFAULT) { + const char *preallocation; + + preallocation = virStoragePreallocationModeTypeToString(def->preallocation); + if (!preallocation) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unexpected preallocation mode")); + goto cleanup; + } + virBufferAsprintf(&buf," <preallocation mode='%s'/>\n", preallocation); + } + if (virStorageVolTargetDefFormat(options, &buf, &def->target, "target") < 0) goto cleanup; diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 9222c4a..c7c7af0 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -98,6 +98,7 @@ struct _virStorageVolDef { virStorageVolSource source; virStorageVolTarget target; virStorageVolTarget backingStore; + int preallocation; /* virStoragePreallocationMode enum */ }; typedef struct _virStorageVolDefList virStorageVolDefList; diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 530071e..63abe65 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -47,6 +47,10 @@ VIR_ENUM_IMPL(virStorageFileFormat, "cloop", "cow", "dmg", "iso", "qcow", "qcow2", "qed", "vmdk", "vpc") +VIR_ENUM_IMPL(virStoragePreallocationMode, + VIR_STORAGE_PREALLOCATION_LAST, + "default", "off", "metadata") + enum lv_endian { LV_LITTLE_ENDIAN = 1, /* 1234 */ LV_BIG_ENDIAN /* 4321 */ diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 13d0e87..3e08163 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -46,6 +46,16 @@ enum virStorageFileFormat { VIR_ENUM_DECL(virStorageFileFormat); +enum virStoragePreallocationMode { + VIR_STORAGE_PREALLOCATION_DEFAULT, + VIR_STORAGE_PREALLOCATION_OFF, + VIR_STORAGE_PREALLOCATION_METADATA, + + VIR_STORAGE_PREALLOCATION_LAST +}; + +VIR_ENUM_DECL(virStoragePreallocationMode); + typedef struct _virStorageFileMetadata { char *backingStore; int backingStoreFormat; diff --git a/tests/storagevolxml2xmlin/vol-qcow2.xml b/tests/storagevolxml2xmlin/vol-qcow2.xml index b4924de..b4c6522 100644 --- a/tests/storagevolxml2xmlin/vol-qcow2.xml +++ b/tests/storagevolxml2xmlin/vol-qcow2.xml @@ -5,6 +5,7 @@ </source> <capacity unit="G">5</capacity> <allocation>294912</allocation> + <preallocation mode='metadata'/> <target> <path>/var/lib/libvirt/images/OtherDemo.img</path> <format type='qcow2'/> diff --git a/tests/storagevolxml2xmlout/vol-qcow2.xml b/tests/storagevolxml2xmlout/vol-qcow2.xml index 4490931..311c52e 100644 --- a/tests/storagevolxml2xmlout/vol-qcow2.xml +++ b/tests/storagevolxml2xmlout/vol-qcow2.xml @@ -5,6 +5,7 @@ </source> <capacity unit='bytes'>5368709120</capacity> <allocation unit='bytes'>294912</allocation> + <preallocation mode='metadata'/> <target> <path>/var/lib/libvirt/images/OtherDemo.img</path> <format type='qcow2'/> -- 1.7.10.1

On 05/17/2012 03:35 PM, Marc-André Lureau wrote:
Allow to specify preallocation mode for QCOW2 images. If not specified or not available, it's ignored.
This change only modify the schema, doc, parsing and tests. --- docs/formatstorage.html.in | 7 +++++++ docs/schemas/storagevol.rng | 18 ++++++++++++++++++ src/conf/storage_conf.c | 26 ++++++++++++++++++++++++++ src/conf/storage_conf.h | 1 + src/util/storage_file.c | 4 ++++ src/util/storage_file.h | 10 ++++++++++ tests/storagevolxml2xmlin/vol-qcow2.xml | 1 + tests/storagevolxml2xmlout/vol-qcow2.xml | 1 + 8 files changed, 68 insertions(+)
Meta-question - is pre-allocation something that is persistent with the existence of the storage volume, or is it something that is one-shot at the creation of the volume? If pre-allocation is a persistent property of the volume itself, then I would expect qemu-img to tell me whether an image is currently pre-allocated, as well as having knobs to tweak to force an image to become pre-allocated where it was not previously in that state. The converse direction, going from pre-allocated to sparse as a form of compression, might also be possible. If this is the case, then making pre-allocation part of the XML for describing a storage volume makes sense. On the other hand, if pre-allocation is only a knob to creation, but once the image is created it is no longer possible to tell whether it was created sparse or pre-allocated, nor is it possible to tweak the image to change between the two states at will, then it makes more sense to add a flag to the creation methods that specify whether to request pre-allocation, and to leave it out of the XML. I need to know the answer to that meta-question before I can review this part of the patch series. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Fri, May 18, 2012 at 12:13 AM, Eric Blake <eblake@redhat.com> wrote:
Meta-question - is pre-allocation something that is persistent with the existence of the storage volume, or is it something that is one-shot at the creation of the volume?
From what I gather, there doesn't seem to be any flag telling you if the qcow2 was preallocated, although I guess there should be a way to somehow guess that.
I think I understand your point, and you may be right that a flag at creation time might be better. But, the fact that we can't easily know from the img itself if it was preallocated is also a loss of information. For example, if a user-friendly client wanted to warn the user of potential slowness due to a qcow2 image created without preallocation, it would be nice to keep that information somewhere, or perhaps implement that "guessing" at the "qemu-img info" level. But if we go that way, then why don't we query the image for all the data it has instead of storing them in the XML, such as capacity? Isn't allocation also a creation-time only value? -- Marc-André Lureau

On 05/17/2012 04:59 PM, Marc-André Lureau wrote:
On Fri, May 18, 2012 at 12:13 AM, Eric Blake <eblake@redhat.com> wrote:
Meta-question - is pre-allocation something that is persistent with the existence of the storage volume, or is it something that is one-shot at the creation of the volume?
From what I gather, there doesn't seem to be any flag telling you if the qcow2 was preallocated, although I guess there should be a way to somehow guess that.
I think I understand your point, and you may be right that a flag at creation time might be better. But, the fact that we can't easily know from the img itself if it was preallocated is also a loss of information. For example, if a user-friendly client wanted to warn the user of potential slowness due to a qcow2 image created without preallocation, it would be nice to keep that information somewhere, or perhaps implement that "guessing" at the "qemu-img info" level. But if we go that way, then why don't we query the image for all the data it has instead of storing them in the XML, such as capacity? Isn't allocation also a creation-time only value?
We _do_ query the image for all the data it has. Capacity and allocation _can_ change at runtime, and when they do, the storage volume information _should_ reflect those changes. That is, we don't store the XML directly on disk, so much as regenerate it on the fly every time someone asks for an API dealing with a storage volume. Yeah, we're not the best at it right now (we need a 'virsh pool-refresh' command to tell us to re-read image properties, and worse, reading properties of a qcow2 file _while_ a qemu process is also modifying the file is dangerous), I'm hoping to improve that by adding better mapping between domains and their storage volumes, and by making a storage volume that is in use by an active domain defer to the domain counterpart command for determining image usage. Back to the question at hand - if there is a way to tell from qemu-img if we have pivoted an image that was originally created without preallocation into one that now uses the full space guaranteed as if it had been pre-allocated, then that would indeed be useful information to reflect back to the user in XML. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Use preallocation mode specified in volume XML format when running qemu-img. --- src/storage/storage_backend.c | 70 +++++++++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index caac2f8..8b31823 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -645,6 +645,38 @@ cleanup: return ret; } +static char * +virStorageQemuImgOptionsStr(virStorageVolDefPtr vol) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool do_encryption = (vol->target.encryption != NULL); + const char *backingType = vol->backingStore.path ? + virStorageFileFormatTypeToString(vol->backingStore.format) : NULL; + bool comma = false; + const char *preallocation = vol->preallocation != VIR_STORAGE_PREALLOCATION_NONE ? + virStoragePreallocationModeTypeToString(vol->preallocation) : NULL; + + if (backingType) { + virBufferAddLit(&buf, "backing_fmt="); + virBufferAdd(&buf, backingType, -1); + comma = true; + } + + if (do_encryption) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "encryption=on"); + comma = true; + } + + if (preallocation) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "preallocation="); + virBufferAdd(&buf, preallocation, -1); + comma = true; + } + + return virBufferContentAndReset(&buf); +} static int virStorageBackendCreateQemuImg(virConnectPtr conn, @@ -658,7 +690,9 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, int imgformat = -1; virCommandPtr cmd = NULL; bool do_encryption = (vol->target.encryption != NULL); + bool with_preallocation = (vol->preallocation != VIR_STORAGE_PREALLOCATION_NONE); unsigned long long int size_arg; + char *options; virCheckFlags(0, -1); @@ -689,6 +723,12 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, return -1; } + if (vol->target.format != VIR_STORAGE_FILE_QCOW2 && with_preallocation) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("Preallocation is only available with qcow2")); + return -1; + } + if (vol->backingStore.path) { int accessRetCode = -1; char *absolutePath = NULL; @@ -781,16 +821,22 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, if (imgformat < 0) goto cleanup; + if (imgformat != QEMU_IMG_BACKING_FORMAT_OPTIONS && with_preallocation) + VIR_INFO("Preallocation not supported with this version of qemu-img"); + cmd = virCommandNew(create_tool); if (inputvol) { virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, inputPath, vol->target.path, NULL); - if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { + virCommandAddArg(cmd, "-o"); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); + VIR_FREE(options); + } else { + if (do_encryption) { virCommandAddArg(cmd, "-e"); } } @@ -811,10 +857,11 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, case QEMU_IMG_BACKING_FORMAT_OPTIONS: virCommandAddArg(cmd, "-o"); - virCommandAddArgFormat(cmd, "backing_fmt=%s%s", backingType, - do_encryption ? ",encryption=on" : ""); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); virCommandAddArg(cmd, vol->target.path); virCommandAddArgFormat(cmd, "%lluK", size_arg); + VIR_FREE(options); break; default: @@ -831,10 +878,13 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, vol->target.path, NULL); virCommandAddArgFormat(cmd, "%lluK", size_arg); - if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { + virCommandAddArg(cmd, "-o"); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); + VIR_FREE(options); + } else { + if (do_encryption) { virCommandAddArg(cmd, "-e"); } } -- 1.7.10.1

On Wed, May 16, 2012 at 12:55:13AM +0200, Marc-André Lureau wrote:
Use preallocation mode specified in volume XML format when running qemu-img. --- src/storage/storage_backend.c | 70 +++++++++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 10 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index caac2f8..8b31823 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -645,6 +645,38 @@ cleanup: return ret; }
+static char * +virStorageQemuImgOptionsStr(virStorageVolDefPtr vol) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool do_encryption = (vol->target.encryption != NULL); + const char *backingType = vol->backingStore.path ? + virStorageFileFormatTypeToString(vol->backingStore.format) : NULL; + bool comma = false; + const char *preallocation = vol->preallocation != VIR_STORAGE_PREALLOCATION_NONE ? + virStoragePreallocationModeTypeToString(vol->preallocation) : NULL; + + if (backingType) { + virBufferAddLit(&buf, "backing_fmt="); + virBufferAdd(&buf, backingType, -1); + comma = true; + } + + if (do_encryption) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "encryption=on"); + comma = true; + } + + if (preallocation) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "preallocation="); + virBufferAdd(&buf, preallocation, -1); + comma = true; + } + + return virBufferContentAndReset(&buf); +}
static int virStorageBackendCreateQemuImg(virConnectPtr conn, @@ -658,7 +690,9 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, int imgformat = -1; virCommandPtr cmd = NULL; bool do_encryption = (vol->target.encryption != NULL); + bool with_preallocation = (vol->preallocation != VIR_STORAGE_PREALLOCATION_NONE); unsigned long long int size_arg; + char *options;
virCheckFlags(0, -1);
@@ -689,6 +723,12 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, return -1; }
+ if (vol->target.format != VIR_STORAGE_FILE_QCOW2 && with_preallocation) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("Preallocation is only available with qcow2"));
This is not an internal error since it's caused by user xml input. The rest of the code seems to be using VIR_ERR_CONFIG_UNSUPPORTED in similar cases. ACK with that fixed. Christophe

Use preallocation mode specified in volume XML format when running qemu-img. --- src/storage/storage_backend.c | 70 +++++++++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index caac2f8..523b6e2 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -645,6 +645,38 @@ cleanup: return ret; } +static char * +virStorageQemuImgOptionsStr(virStorageVolDefPtr vol) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool do_encryption = (vol->target.encryption != NULL); + const char *backingType = vol->backingStore.path ? + virStorageFileFormatTypeToString(vol->backingStore.format) : NULL; + bool comma = false; + const char *preallocation = vol->preallocation != VIR_STORAGE_PREALLOCATION_NONE ? + virStoragePreallocationModeTypeToString(vol->preallocation) : NULL; + + if (backingType) { + virBufferAddLit(&buf, "backing_fmt="); + virBufferAdd(&buf, backingType, -1); + comma = true; + } + + if (do_encryption) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "encryption=on"); + comma = true; + } + + if (preallocation) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "preallocation="); + virBufferAdd(&buf, preallocation, -1); + comma = true; + } + + return virBufferContentAndReset(&buf); +} static int virStorageBackendCreateQemuImg(virConnectPtr conn, @@ -658,7 +690,9 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, int imgformat = -1; virCommandPtr cmd = NULL; bool do_encryption = (vol->target.encryption != NULL); + bool with_preallocation = (vol->preallocation != VIR_STORAGE_PREALLOCATION_NONE); unsigned long long int size_arg; + char *options; virCheckFlags(0, -1); @@ -689,6 +723,12 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, return -1; } + if (vol->target.format != VIR_STORAGE_FILE_QCOW2 && with_preallocation) { + virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Preallocation is only available with qcow2")); + return -1; + } + if (vol->backingStore.path) { int accessRetCode = -1; char *absolutePath = NULL; @@ -781,16 +821,22 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, if (imgformat < 0) goto cleanup; + if (imgformat != QEMU_IMG_BACKING_FORMAT_OPTIONS && with_preallocation) + VIR_INFO("Preallocation not supported with this version of qemu-img"); + cmd = virCommandNew(create_tool); if (inputvol) { virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, inputPath, vol->target.path, NULL); - if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { + virCommandAddArg(cmd, "-o"); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); + VIR_FREE(options); + } else { + if (do_encryption) { virCommandAddArg(cmd, "-e"); } } @@ -811,10 +857,11 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, case QEMU_IMG_BACKING_FORMAT_OPTIONS: virCommandAddArg(cmd, "-o"); - virCommandAddArgFormat(cmd, "backing_fmt=%s%s", backingType, - do_encryption ? ",encryption=on" : ""); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); virCommandAddArg(cmd, vol->target.path); virCommandAddArgFormat(cmd, "%lluK", size_arg); + VIR_FREE(options); break; default: @@ -831,10 +878,13 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, vol->target.path, NULL); virCommandAddArgFormat(cmd, "%lluK", size_arg); - if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { + virCommandAddArg(cmd, "-o"); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); + VIR_FREE(options); + } else { + if (do_encryption) { virCommandAddArg(cmd, "-e"); } } -- 1.7.10.1

ACK On Wed, May 16, 2012 at 01:21:33PM +0200, Marc-André Lureau wrote:
Use preallocation mode specified in volume XML format when running qemu-img. --- src/storage/storage_backend.c | 70 +++++++++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 10 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index caac2f8..523b6e2 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -645,6 +645,38 @@ cleanup: return ret; }
+static char * +virStorageQemuImgOptionsStr(virStorageVolDefPtr vol) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool do_encryption = (vol->target.encryption != NULL); + const char *backingType = vol->backingStore.path ? + virStorageFileFormatTypeToString(vol->backingStore.format) : NULL; + bool comma = false; + const char *preallocation = vol->preallocation != VIR_STORAGE_PREALLOCATION_NONE ? + virStoragePreallocationModeTypeToString(vol->preallocation) : NULL; + + if (backingType) { + virBufferAddLit(&buf, "backing_fmt="); + virBufferAdd(&buf, backingType, -1); + comma = true; + } + + if (do_encryption) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "encryption=on"); + comma = true; + } + + if (preallocation) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "preallocation="); + virBufferAdd(&buf, preallocation, -1); + comma = true; + } + + return virBufferContentAndReset(&buf); +}
static int virStorageBackendCreateQemuImg(virConnectPtr conn, @@ -658,7 +690,9 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, int imgformat = -1; virCommandPtr cmd = NULL; bool do_encryption = (vol->target.encryption != NULL); + bool with_preallocation = (vol->preallocation != VIR_STORAGE_PREALLOCATION_NONE); unsigned long long int size_arg; + char *options;
virCheckFlags(0, -1);
@@ -689,6 +723,12 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, return -1; }
+ if (vol->target.format != VIR_STORAGE_FILE_QCOW2 && with_preallocation) { + virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Preallocation is only available with qcow2")); + return -1; + } + if (vol->backingStore.path) { int accessRetCode = -1; char *absolutePath = NULL; @@ -781,16 +821,22 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, if (imgformat < 0) goto cleanup;
+ if (imgformat != QEMU_IMG_BACKING_FORMAT_OPTIONS && with_preallocation) + VIR_INFO("Preallocation not supported with this version of qemu-img"); + cmd = virCommandNew(create_tool);
if (inputvol) { virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, inputPath, vol->target.path, NULL);
- if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { + virCommandAddArg(cmd, "-o"); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); + VIR_FREE(options); + } else { + if (do_encryption) { virCommandAddArg(cmd, "-e"); } } @@ -811,10 +857,11 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
case QEMU_IMG_BACKING_FORMAT_OPTIONS: virCommandAddArg(cmd, "-o"); - virCommandAddArgFormat(cmd, "backing_fmt=%s%s", backingType, - do_encryption ? ",encryption=on" : ""); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); virCommandAddArg(cmd, vol->target.path); virCommandAddArgFormat(cmd, "%lluK", size_arg); + VIR_FREE(options); break;
default: @@ -831,10 +878,13 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, vol->target.path, NULL); virCommandAddArgFormat(cmd, "%lluK", size_arg);
- if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { + virCommandAddArg(cmd, "-o"); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); + VIR_FREE(options); + } else { + if (do_encryption) { virCommandAddArg(cmd, "-e"); } } -- 1.7.10.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Use preallocation mode specified in volume XML format when running qemu-img. --- src/storage/storage_backend.c | 70 +++++++++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index caac2f8..faea2c5 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -645,6 +645,38 @@ cleanup: return ret; } +static char * +virStorageQemuImgOptionsStr(virStorageVolDefPtr vol) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool do_encryption = (vol->target.encryption != NULL); + const char *backingType = vol->backingStore.path ? + virStorageFileFormatTypeToString(vol->backingStore.format) : NULL; + bool comma = false; + const char *preallocation = vol->preallocation != VIR_STORAGE_PREALLOCATION_DEFAULT ? + virStoragePreallocationModeTypeToString(vol->preallocation) : NULL; + + if (backingType) { + virBufferAddLit(&buf, "backing_fmt="); + virBufferAdd(&buf, backingType, -1); + comma = true; + } + + if (do_encryption) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "encryption=on"); + comma = true; + } + + if (preallocation) { + virBufferAdd(&buf, comma ? "," : "", -1); + virBufferAddLit(&buf, "preallocation="); + virBufferAdd(&buf, preallocation, -1); + comma = true; + } + + return virBufferContentAndReset(&buf); +} static int virStorageBackendCreateQemuImg(virConnectPtr conn, @@ -658,7 +690,9 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, int imgformat = -1; virCommandPtr cmd = NULL; bool do_encryption = (vol->target.encryption != NULL); + bool with_preallocation = (vol->preallocation != VIR_STORAGE_PREALLOCATION_DEFAULT); unsigned long long int size_arg; + char *options; virCheckFlags(0, -1); @@ -689,6 +723,12 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, return -1; } + if (vol->target.format != VIR_STORAGE_FILE_QCOW2 && with_preallocation) { + virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Preallocation is only available with qcow2")); + return -1; + } + if (vol->backingStore.path) { int accessRetCode = -1; char *absolutePath = NULL; @@ -781,16 +821,22 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, if (imgformat < 0) goto cleanup; + if (imgformat != QEMU_IMG_BACKING_FORMAT_OPTIONS && with_preallocation) + VIR_INFO("Preallocation not supported with this version of qemu-img"); + cmd = virCommandNew(create_tool); if (inputvol) { virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, inputPath, vol->target.path, NULL); - if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { + virCommandAddArg(cmd, "-o"); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); + VIR_FREE(options); + } else { + if (do_encryption) { virCommandAddArg(cmd, "-e"); } } @@ -811,10 +857,11 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, case QEMU_IMG_BACKING_FORMAT_OPTIONS: virCommandAddArg(cmd, "-o"); - virCommandAddArgFormat(cmd, "backing_fmt=%s%s", backingType, - do_encryption ? ",encryption=on" : ""); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); virCommandAddArg(cmd, vol->target.path); virCommandAddArgFormat(cmd, "%lluK", size_arg); + VIR_FREE(options); break; default: @@ -831,10 +878,13 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, vol->target.path, NULL); virCommandAddArgFormat(cmd, "%lluK", size_arg); - if (do_encryption) { - if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - virCommandAddArgList(cmd, "-o", "encryption=on", NULL); - } else { + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { + virCommandAddArg(cmd, "-o"); + options = virStorageQemuImgOptionsStr(vol); + virCommandAddArg(cmd, options); + VIR_FREE(options); + } else { + if (do_encryption) { virCommandAddArg(cmd, "-e"); } } -- 1.7.10.1

Add element <forward> to add TCP or UDP port redirection from host to guest in user mode networking. --- docs/formatdomain.html.in | 25 ++++ docs/schemas/domaincommon.rng | 29 ++++ src/conf/domain_conf.c | 168 ++++++++++++++++++++++ src/conf/domain_conf.h | 25 ++++ tests/qemuargv2xmltest.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 2 + 6 files changed, 251 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3875167..255e91b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2268,6 +2268,31 @@ VMs to have outgoing access. </p> + <p> + <span class="since">Since 0.9.13</span>, port redirections from + host to guest can be added by providing <code>forward</code> + elements that takes the following attributes: + </p> + + <dl> + <dt><code>protocol</code></dt> + <dd>Either <code>tcp</code> (default) or <code>udp</code>.</dd> + + <dt><code>hostport</code></dt> + <dd>Host port to redirect.</dd> + + <dt><code>guestport</code></dt> + <dd>Guest port to redirect to.</dd> + + <dt><code>hostipaddr</code></dt> + <dd>IPv4 address to bound the redirection to a specific host + interface.</dd> + + <dt><code>guestipaddr</code></dt> + <dd>IPv4 address to bound the redirection to a specific guest + interface.</dd> + </dl> + <pre> ... <devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 34f63c3..adb2c3e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1408,6 +1408,35 @@ <value>user</value> </attribute> <interleave> + <zeroOrMore> + <element name="forward"> + <attribute name="hostport"> + <ref name="PortNumber"/> + </attribute> + <attribute name="guestport"> + <ref name="PortNumber"/> + </attribute> + <optional> + <attribute name="protocol"> + <choice> + <value>tcp</value> + <value>udp</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="hostaddr"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="guestaddr"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <empty/> + </element> + </zeroOrMore> <ref name="interface-options"/> </interleave> </group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 78755cf..fb0ff62 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -650,6 +650,11 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, "static", "auto"); + +VIR_ENUM_IMPL(virDomainNetForwardProtocol, VIR_DOMAIN_NET_FORWARD_PROTOCOL_LAST, + "tcp", + "udp") + #define virDomainReportError(code, ...) \ virReportErrorHelper(VIR_FROM_DOMAIN, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) @@ -1019,8 +1024,19 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) VIR_FREE(def); } +void +virDomainNetForwardDefFree(virDomainNetForwardDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def); +} + void virDomainNetDefFree(virDomainNetDefPtr def) { + int i; + if (!def) return; @@ -1066,6 +1082,11 @@ void virDomainNetDefFree(virDomainNetDefPtr def) break; case VIR_DOMAIN_NET_TYPE_USER: + for (i = 0; i < def->data.user.nforward; i++) + virDomainNetForwardDefFree(def->data.user.forwards[i]); + VIR_FREE(def->data.user.forwards); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -4380,6 +4401,81 @@ error: return ret; } +static virDomainNetForwardDefPtr +virDomainNetForwardDefParseXML(const xmlNodePtr node) +{ + char *protocol = NULL; + char *hostaddr = NULL; + char *guestaddr = NULL; + char *hostport = NULL; + char *guestport = NULL; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + protocol = virXMLPropString(node, "protocol"); + if (protocol == NULL) { + def->protocol = VIR_DOMAIN_NET_FORWARD_PROTOCOL_TCP; + } else if ((def->protocol = virDomainNetForwardProtocolTypeFromString(protocol)) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("unknown forward protocol '%s'"), protocol); + goto error; + } + + hostport = virXMLPropString(node, "hostport"); + if (!hostport || + virStrToLong_i(hostport, NULL, 10, &def->hostport) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("Cannot parse <forward> 'hostport' attribute")); + goto error; + } + + guestport = virXMLPropString(node, "guestport"); + if (!guestport || + virStrToLong_i(guestport, NULL, 10, &def->guestport) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("Cannot parse <forward> 'guestport' attribute")); + goto error; + } + + hostaddr = virXMLPropString(node, "hostaddr"); + if (hostaddr) { + if (virSocketAddrParse(&def->hostaddr, hostaddr, AF_INET) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("Bad host address '%s' for redirection"), hostaddr); + goto error; + } + def->has_hostaddr = true; + } + + guestaddr = virXMLPropString(node, "guestaddr"); + if (guestaddr) { + if (virSocketAddrParse(&def->guestaddr, guestaddr, AF_INET) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("Bad guest address '%s' for redirection"), guestaddr); + goto error; + } + def->has_guestaddr = true; + } + +cleanup: + VIR_FREE(protocol); + VIR_FREE(hostaddr); + VIR_FREE(guestaddr); + VIR_FREE(hostport); + VIR_FREE(guestport); + + return def; + +error: + virDomainNetForwardDefFree(def); + def = NULL; + goto cleanup; +} + #define NET_MODEL_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_-" @@ -4423,6 +4519,8 @@ virDomainNetDefParseXML(virCapsPtr caps, virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; int ret; + int nforwards; + xmlNodePtr *forwardNodes = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -4712,6 +4810,28 @@ virDomainNetDefParseXML(virCapsPtr caps, break; case VIR_DOMAIN_NET_TYPE_USER: + /* parse the <forward> elements */ + nforwards = virXPathNodeSet("./forward", ctxt, &forwardNodes); + if (nforwards < 0) + goto error; + + if (nforwards > 0) { + int i; + if (VIR_ALLOC_N(def->data.user.forwards, nforwards) < 0) { + virReportOOMError(); + goto error; + } + for (i = 0; i < nforwards; i++) { + virDomainNetForwardDefPtr fwd = + virDomainNetForwardDefParseXML(forwardNodes[i]); + if (fwd == NULL) + goto error; + def->data.user.forwards[def->data.user.nforward++] = fwd; + } + VIR_FREE(forwardNodes); + } + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -4824,6 +4944,7 @@ virDomainNetDefParseXML(virCapsPtr caps, cleanup: ctxt->node = oldnode; + VIR_FREE(forwardNodes); VIR_FREE(macaddr); VIR_FREE(network); VIR_FREE(portgroup); @@ -11446,11 +11567,50 @@ error: } static int +virDomainNetForwardDefFormat(virBufferPtr buf, + virDomainNetForwardDefPtr def) +{ + const char *protocol; + char *ip; + + if (!def) + return 0; + + protocol = virDomainNetForwardProtocolTypeToString(def->protocol); + if (!protocol) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected net type %d"), def->protocol); + return -1; + } + virBufferAsprintf(buf, "<forward protocol='%s'", protocol); + + if (def->has_hostaddr) { + ip = virSocketAddrFormat(&def->hostaddr); + virBufferAsprintf(buf, " hostaddr='%s'", ip); + VIR_FREE(ip); + } + + virBufferAsprintf(buf, " hostport='%d'", def->hostport); + + if (def->has_guestaddr) { + ip = virSocketAddrFormat(&def->guestaddr); + virBufferAsprintf(buf, " guestaddr='%s'", ip); + VIR_FREE(ip); + } + + virBufferAsprintf(buf, " guestport='%d'", def->guestport); + + virBufferAddLit(buf, "/>\n"); + return 0; +} + +static int virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, unsigned int flags) { const char *type = virDomainNetTypeToString(def->type); + int i; if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -11550,6 +11710,14 @@ virDomainNetDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_NET_TYPE_USER: + virBufferAdjustIndent(buf, 6); + for (i = 0; i < def->data.user.nforward; i++) { + if (virDomainNetForwardDefFormat(buf, def->data.user.forwards[i]) < 0) + return -1; + } + virBufferAdjustIndent(buf, -6); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4c56902..be42f4f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -772,6 +772,25 @@ struct _virDomainActualNetDef { virNetDevBandwidthPtr bandwidth; }; +enum virDomainNetForwardProtocol { + VIR_DOMAIN_NET_FORWARD_PROTOCOL_TCP, + VIR_DOMAIN_NET_FORWARD_PROTOCOL_UDP, + + VIR_DOMAIN_NET_FORWARD_PROTOCOL_LAST, +}; + +typedef struct _virDomainNetForwardDef virDomainNetForwardDef; +typedef virDomainNetForwardDef *virDomainNetForwardDefPtr; +struct _virDomainNetForwardDef { + int protocol; /* enum virDomainNetForwardProtocol */ + bool has_hostaddr; + virSocketAddr hostaddr; + int hostport; + bool has_guestaddr; + virSocketAddr guestaddr; + int guestport; +}; + /* Stores the virtual network interface configuration */ struct _virDomainNetDef { enum virDomainNetType type; @@ -825,6 +844,10 @@ struct _virDomainNetDef { virDomainHostdevDef def; virNetDevVPortProfilePtr virtPortProfile; } hostdev; + struct { + int nforward; + virDomainNetForwardDefPtr *forwards; + } user; } data; struct { bool sndbuf_specified; @@ -1860,6 +1883,7 @@ int virDomainDiskFindControllerModel(virDomainDefPtr def, void virDomainControllerDefFree(virDomainControllerDefPtr def); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainActualNetDefFree(virDomainActualNetDefPtr def); +void virDomainNetForwardDefFree(virDomainNetForwardDefPtr def); void virDomainNetDefFree(virDomainNetDefPtr def); void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def); void virDomainChrDefFree(virDomainChrDefPtr def); @@ -2174,6 +2198,7 @@ VIR_ENUM_DECL(virDomainFSAccessMode) VIR_ENUM_DECL(virDomainFSWrpolicy) VIR_ENUM_DECL(virDomainNet) VIR_ENUM_DECL(virDomainNetBackend) +VIR_ENUM_DECL(virDomainNetForwardProtocol) VIR_ENUM_DECL(virDomainNetVirtioTxMode) VIR_ENUM_DECL(virDomainNetInterfaceLinkState) VIR_ENUM_DECL(virDomainChrDevice) diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 439218e..cf2862b 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -207,7 +207,8 @@ mymain(void) DO_TEST("misc-acpi"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); - DO_TEST("net-user"); + /* Fixed in following commit */ + /* DO_TEST("net-user"); */ DO_TEST("net-virtio"); DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml index 37e5edf..c018d34 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml @@ -23,6 +23,8 @@ <controller type='ide' index='0'/> <interface type='user'> <mac address='00:11:22:33:44:55'/> + <forward protocol='tcp' hostport='2222' guestport='22'/> + <forward protocol='udp' hostaddr='127.0.0.1' hostport='2242' guestaddr='10.0.2.15' guestport='42'/> </interface> <memballoon model='virtio'/> </devices> -- 1.7.10.1

Implement port forwarding for user mode networking with QEMU. --- src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 139 +++++++++++++++++++++ tests/qemuargv2xmltest.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 5 +- 4 files changed, 146 insertions(+), 4 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7d09f33..d497bce 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -396,6 +396,9 @@ virDomainNetInsert; virDomainNetRemove; virDomainNetRemoveByMac; virDomainNetTypeToString; +virDomainNetForwardDefFree; +virDomainNetForwardProtocolTypeFromString; +virDomainNetForwardProtocolTypeToString; virDomainNostateReasonTypeFromString; virDomainNostateReasonTypeToString; virDomainNumatuneMemModeTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9f99dce..2c9d948 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2917,6 +2917,34 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, virBufferAsprintf(&buf, ",sndbuf=%lu", net->tune.sndbuf); } + if (netType == VIR_DOMAIN_NET_TYPE_USER) { + int i; + + for (i = 0; i < net->data.user.nforward; ++i) { + char *hostaddr = NULL; + char *guestaddr = NULL; + virDomainNetForwardDefPtr fwd = net->data.user.forwards[i]; + const char *protocol = virDomainNetForwardProtocolTypeToString(fwd->protocol); + if (!protocol) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Invalid forward protocol")); + return NULL; + } + + if (fwd->has_hostaddr) + hostaddr = virSocketAddrFormat(&fwd->hostaddr); + if (fwd->has_guestaddr) + guestaddr = virSocketAddrFormat(&fwd->guestaddr); + + virBufferAsprintf(&buf, ",hostfwd=%s:%s:%d-%s:%d", protocol, + hostaddr ? hostaddr : "", fwd->hostport, + guestaddr ? guestaddr : "", fwd->guestport); + + VIR_FREE(hostaddr); + VIR_FREE(guestaddr); + } + } + if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); @@ -6785,6 +6813,94 @@ qemuFindNICForVLAN(int nnics, } +static virDomainNetForwardDefPtr +qemuParseNetForward(char *val) +{ + char *protocol, *host, *guest, *port; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + protocol = val; + host = strchr(protocol, ':'); + if (!host) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse hostfwd host '%s'"), val); + goto error; + } + *host = '\0'; + host++; + guest = strchr(host, '-'); + if (!guest) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse hostfwd guest '%s'"), val); + goto error; + } + *guest = '\0'; + guest++; + + if (STREQ(protocol, "")) { + def->protocol = VIR_DOMAIN_NET_FORWARD_PROTOCOL_TCP; + } else if ((def->protocol = virDomainNetForwardProtocolTypeFromString(protocol)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown forward protocol '%s'"), protocol); + goto error; + } + + port = strchr(host, ':'); + if (!port) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing host port")); + goto error; + } + *port = '\0'; + port++; + if (!STREQ(host, "")) { + if (virSocketAddrParse(&def->hostaddr, host, AF_INET) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Bad host address '%s' for redirection"), host); + goto error; + } + def->has_hostaddr = true; + } + if (virStrToLong_i(port, NULL, 10, &def->hostport) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse host port")); + goto error; + } + + port = strchr(guest, ':'); + if (!port) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing guest port")); + goto error; + } + *port = '\0'; + port++; + if (!STREQ(guest, "")) { + if (virSocketAddrParse(&def->guestaddr, guest, AF_INET) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Bad guest address '%s' for redirection"), guest); + goto error; + } + def->has_guestaddr = true; + } + if (virStrToLong_i(port, NULL, 10, &def->guestport) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse guest port")); + goto error; + } + + return def; + +error: + virDomainNetForwardDefFree(def); + return NULL; +} + /* * Tries to parse a QEMU -net backend argument. Gets given * a list of all known -net frontend arguments to try and @@ -6804,6 +6920,7 @@ qemuParseCommandLineNet(virCapsPtr caps, int wantvlan = 0; const char *tmp; int genmac = 1; + int nforward = 0; int i; tmp = strchr(val, ','); @@ -6850,9 +6967,31 @@ qemuParseCommandLineNet(virCapsPtr caps, STREQ(keywords[i], "ifname")) { def->ifname = values[i]; values[i] = NULL; + } else if (def->type == VIR_DOMAIN_NET_TYPE_USER && + STREQ(keywords[i], "hostfwd")) { + nforward++; } } + if (nforward > 0) { + if (VIR_ALLOC_N(def->data.user.forwards, nforward) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < nkeywords ; i++) { + virDomainNetForwardDefPtr fwd; + if (def->type != VIR_DOMAIN_NET_TYPE_USER || + !STREQ(keywords[i], "hostfwd")) + continue; + + if ((fwd = qemuParseNetForward(values[i])) == NULL) + goto cleanup; + + def->data.user.forwards[def->data.user.nforward] = fwd; + def->data.user.nforward++; + } + } /* Done parsing the nic backend. Now to try and find corresponding * frontend, based off vlan number. NB this assumes a 1-1 mapping diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index cf2862b..439218e 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -207,8 +207,7 @@ mymain(void) DO_TEST("misc-acpi"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); - /* Fixed in following commit */ - /* DO_TEST("net-user"); */ + DO_TEST("net-user"); DO_TEST("net-virtio"); DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args index 093ff01..db31e95 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net user,vlan=0 -serial none -parallel none \ --usb +macaddr=00:11:22:33:44:55,vlan=0 \ +-net user,vlan=0,hostfwd=tcp::2222-:22,hostfwd=udp:127.0.0.1:2242-10.0.2.15:42 \ +-serial none -parallel none -usb -- 1.7.10.1

On 05/15/2012 04:55 PM, Marc-André Lureau wrote:
--- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemuhelptest.c | 3 ++- 3 files changed, 8 insertions(+), 1 deletion(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Christophe Fergeau
-
Eric Blake
-
Marc-André Lureau