In subject:
s/Fix/conf: Fix/
On 05/16/14 15:23, Ján Tomko wrote:
> We allow a seclabel to be specified in the <source> element
> of a chardev:
>
> <serial type='file'>
> <source path='/tmp/serial.file'>
> <seclabel model='dac' relabel='no'/>
> </source>
> </serial>
There is one paragraph mentioning that in the XML format documentation.
I think it would be worth adding (as a separate patch) an example of the
usage too.
>
> But we format it outside the source:
>
> <serial type='file'>
> <source path='/tmp/serial.file'/>
> <target port='0'/>
> <seclabel model='dac' relabel='no'/>
> </serial>
>
> Move the formatting inside the source to fix this to make the
> seclabel persistent across XML format->parse.
>
> Introduced by commit f8b08d0 'Add <seclabel> to character devices.'
> ---
> src/conf/domain_conf.c | 27 +++++++--------
> .../qemuxml2argv-chardev-label.xml | 40 ++++++++++++++++++++++
> tests/qemuxml2xmltest.c | 2 ++
> 3 files changed, 54 insertions(+), 15 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-chardev-label.xml
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 041a113..81e9436 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15862,6 +15862,7 @@ virDomainNetDefFormat(virBufferPtr buf,
> * output at " type='type'>". */
> static int
> virDomainChrSourceDefFormat(virBufferPtr buf,
> + virDomainChrDefPtr chr_def,
> virDomainChrSourceDefPtr def,
> bool tty_compat,
> unsigned int flags)
> @@ -15898,8 +15899,11 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
> if (def->type != VIR_DOMAIN_CHR_TYPE_PTY ||
> (def->data.file.path &&
> !(flags & VIR_DOMAIN_XML_INACTIVE))) {
> - virBufferEscapeString(buf, "<source path='%s'/>\n",
> + virBufferEscapeString(buf, "<source path='%s'",
> def->data.file.path);
> + virDomainSourceDefFormatSeclabel(buf, chr_def->nseclabels,
> + chr_def->seclabels,
> + flags);
> }
> break;
>
I think that "case VIR_DOMAIN_CHR_TYPE_UNIX:" should be probably handled
too.
> @@ -15957,7 +15961,9 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
> virBufferAsprintf(buf, "<source mode='%s'",
> def->data.nix.listen ? "bind" : "connect");
> virBufferEscapeString(buf, " path='%s'", def->data.nix.path);
> - virBufferAddLit(buf, "/>\n");
> + virDomainSourceDefFormatSeclabel(buf, chr_def->nseclabels,
> + chr_def->seclabels,
> + flags);
> break;
>
> case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
> @@ -15979,7 +15985,6 @@ virDomainChrDefFormat(virBufferPtr buf,
> const char *targetType = virDomainChrTargetTypeToString(def->deviceType,
> def->targetType);
> bool tty_compat;
> - size_t n;
>
> int ret = 0;
>
> @@ -15997,7 +16002,7 @@ virDomainChrDefFormat(virBufferPtr buf,
> def->source.type == VIR_DOMAIN_CHR_TYPE_PTY &&
> !(flags & VIR_DOMAIN_XML_INACTIVE) &&
> def->source.data.file.path);
> - if (virDomainChrSourceDefFormat(buf, &def->source, tty_compat, flags) < 0)
> + if (virDomainChrSourceDefFormat(buf, def, &def->source, tty_compat, flags) < 0)
> return -1;
>
> /* Format <target> block */
> @@ -16069,14 +16074,6 @@ virDomainChrDefFormat(virBufferPtr buf,
> return -1;
> }
>
> - /* Security label overrides, if any. */
> - if (def->seclabels && def->nseclabels > 0) {
> - virBufferAdjustIndent(buf, 2);
> - for (n = 0; n < def->nseclabels; n++)
> - virSecurityDeviceLabelDefFormat(buf, def->seclabels[n], flags);
> - virBufferAdjustIndent(buf, -2);
> - }
> -
> virBufferAdjustIndent(buf, -2);
> virBufferAsprintf(buf, "</%s>\n", elementName);
>
> @@ -16119,7 +16116,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
> break;
>
> case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> - if (virDomainChrSourceDefFormat(buf, &def->data.passthru, false,
> + if (virDomainChrSourceDefFormat(buf, NULL, &def->data.passthru, false,
> flags) < 0)
Passing NULL as chr_def to virDomainChrSourceDefFormat will induce a
crash once you will try to format a RNG, smartcard or other device with
a chardev backend with type PIPE or other.
> return -1;
> break;
> @@ -16384,7 +16381,7 @@ virDomainRNGDefFormat(virBufferPtr buf,
>
> case VIR_DOMAIN_RNG_BACKEND_EGD:
> virBufferAdjustIndent(buf, 2);
> - if (virDomainChrSourceDefFormat(buf, def->source.chardev,
> + if (virDomainChrSourceDefFormat(buf, NULL, def->source.chardev,
> false, flags) < 0)
> return -1;
> virBufferAdjustIndent(buf, -2);
> @@ -16976,7 +16973,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf,
>
> virBufferAsprintf(buf, "<redirdev bus='%s'", bus);
> virBufferAdjustIndent(buf, 2);
> - if (virDomainChrSourceDefFormat(buf, &def->source.chr, false, flags) < 0)
> + if (virDomainChrSourceDefFormat(buf, NULL, &def->source.chr, false, flags) < 0)
> return -1;
> if (virDomainDeviceInfoFormat(buf, &def->info,
> flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) < 0)
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-chardev-label.xml b/tests/qemuxml2argvdata/qemuxml2argv-chardev-label.xml
> new file mode 100644
> index 0000000..b6df67a
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-chardev-label.xml
> @@ -0,0 +1,40 @@
> +<domain type='qemu'>
> + <name>machine</name>
> + <uuid>2187c512-ff97-47d7-b67c-c02d3bdc219d</uuid>
> + <memory unit='KiB'>219100</memory>
> + <currentMemory unit='KiB'>219100</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='x86_64' 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>
> + <controller type='usb' index='0'/>
> + <controller type='ide' index='0'/>
> + <controller type='pci' index='0' model='pci-root'/>
> + <serial type='file'>
> + <source path='/tmp/serial.file'>
> + <seclabel model='dac' relabel='no'/>
> + </source>
> + <target port='0'/>
> + </serial>
> + <serial type='unix'>
> + <source mode='connect' path='/tmp/serial.sock'>
> + <seclabel model='dac' relabel='no'/>
> + </source>
> + <target port='1'/>
> + </serial>
> + <console type='file'>
> + <source path='/tmp/serial.file'>
> + <seclabel model='dac' relabel='no'/>
> + </source>
> + <target type='serial' port='0'/>
> + </console>
> + <memballoon model='virtio'/>
> + </devices>
> +</domain>
Nice test, but add a device:
<rng model='virtio'>
<backend model='egd' type='pipe'>
<source path='/dev/null'/>
</backend>
</rng>
This will lead with the code in this patch to the crash described above.
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 3ea03e6..da528da 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -362,6 +362,8 @@ mymain(void)
>
> DO_TEST_DIFFERENT("disk-backing-chains");
>
> + DO_TEST("chardev-label");
> +
> virObjectUnref(driver.caps);
> virObjectUnref(driver.xmlopt);
>
>
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list