Re: [RFC PATCH 2/2] conf: Add VM Generation ID device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 20, 2018 at 18:55:44 -0400, John Ferlan wrote:
> Add VM Generation ID device XML schema, parse, format, and documentation.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  docs/formatdomain.html.in                 |  54 ++++++++++++++
>  docs/schemas/domaincommon.rng             |  21 ++++++
>  src/conf/domain_conf.c                    | 112 +++++++++++++++++++++++++++++-
>  src/conf/domain_conf.h                    |  11 +++
>  tests/qemuxml2argvdata/vmgenid-auto.xml   |  32 +++++++++
>  tests/qemuxml2argvdata/vmgenid.xml        |  32 +++++++++
>  tests/qemuxml2xmloutdata/vmgenid-auto.xml |   1 +
>  tests/qemuxml2xmloutdata/vmgenid.xml      |   1 +
>  tests/qemuxml2xmltest.c                   |   3 +
>  9 files changed, 266 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/vmgenid-auto.xml
>  create mode 100644 tests/qemuxml2argvdata/vmgenid.xml
>  create mode 120000 tests/qemuxml2xmloutdata/vmgenid-auto.xml
>  create mode 120000 tests/qemuxml2xmloutdata/vmgenid.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6fd2189cd2..895e51b343 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -8027,6 +8027,60 @@ qemu-kvm -net nic,model=? /dev/null
>        </dd>
>      </dl>
>  
> +    <h4><a id="elementsVmgenid">VM Generation ID device</a></h4>
> +
> +    <p>
> +      <span class="since">Since 4.2.0</span>, the <code>vmgenid</code>
> +      element can be used to add a Virtual Machine Generation ID device.
> +      A <code>vmgenid</code> device is an emulated device which exposes
> +      a 128-bit, cryptographically random, integer value identifier,
> +      referred to as a Globally Unique Identifier, or GUID. The value is
> +      stored within the virtual machine's BIOS so that programs running in

According to the qemu docs, it's stored in the ACPI table, so it does
not depend on BIOS being used. EFI can be used as well.

> +      the virtual machine can protect themselves from potential corruption
> +      by checking that the Generation ID has not changed immediately prior
> +      to committing a transaction.

This last sentence is weird. I'd refrain from giving examples and rather
stick to describe which changes can be tracked by this.


> +
> +      The <code>vmgenid</code> device will update the BIOS entry each time

BIOS ...

> +      the virtual machine executes from a different configuration file such
> +      as executing from a recovered snapshot or executing after restoring

Okay. From this it seems that the GUID should change where the VM state
could be reverted to a point in history of the execution of the guest
software. This basically means that it should be always left to
autogenerating except in cases of migration where the new qemu process
needs to continue to use the old GUID.

This means that you'll need to query it if 'auto' is specified and qemu
does not guarantee that it's properly transported in the migration
stream. On the other hand, if it is transported in the migration stream,
then save/restore and reversion of external snapshots with memory will
be fun, since that uses the migration code.

You'll need to figure out where qemu stores this and how it's handled on
migration, since I did not see it in the doc.

Also there is one other problem and that is reversion of internal
snapshots, which in some cases does not require restarting the emulator
in case of qemu. In such case you'll need to make sure that the
condition is modified so that a VM with this device will always restart
qemu.

> +      from backup. Programs running in a virtual machine can protect themselves
> +      from potential corruption by checking that the generation ID has not
> +      changed immediately prior to committing a transaction, they can also
> +      use the data provided in the 128-bit identifier as a high entropy
> +      random data source.

Whoah. The qemu docs don't speak about entropy source. Is it described
in the Microsoft documentation? In any case, if the GUID is going to be
used as an entropy source, we need to approach it way more carefully.
Especially don't report it to unauthorized eyes. I'd be very happy if
will not have to keep it secret. This will get especially tricky since
there will be cases where we'll need to put it on the command line
(either on migration or on save/restore).

> +    </p>
> +
> +    <p>
> +      Example:
> +    </p>
> +<pre>
> +...
> +&lt;devices&gt;
> +  &lt;vmgenid guid='3e3fce45-4f53-4fa7-bb32-11f34168b82b'/&gt;

So I think we should use a more generic element with 'vmgenid' as a
model. It looks like this is one of possible implementations of such a
thing and it's fully possible to emulate a PCI device or anything else
with similar semantics.

> +&lt;/devices&gt;
> +...
> +</pre>
> +
> +<pre>
> +...
> +&lt;devices&gt;
> +  &lt;vmgenid guid='auto'/&gt;
> +&lt;/devices&gt;
> +...
> +</pre>
> +
> +    <dl>
> +      <dt><code>guid</code></dt>
> +      <dd>
> +        <p>
> +         The required <code>guid</code> attribute can be either a provided
> +         (<a href="#elementsMetadata"><code>uuid</code></a>) formatted value
> +         or the string 'auto' if the underlying hypervisor supports creating

So 'auto' in place of the guid is a qemuism according to the docs. You
should go with a separate element for it in cases where you need to
configure that the setting is automatic, but you also need to transfer
the guid itself.

Also the necessity to allow configuration of the GUID really depends on
the questions I've asked above. It might be even unnecessary to allow
configuration of the GUID itself depending on that. In cases where we'll
need to preserve it accross migration, we'll really need a way to store
it in the XML.

> +         its own value.
> +        </p>
> +      </dd>
> +    </dl>
> +
>      <h3><a id="seclabel">Security label</a></h3>
>  
>      <p>
>        <interleave>

[...]

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2f07180faa..aaba2a47f7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> @@ -15690,6 +15692,45 @@ virDomainIOMMUDefParseXML(xmlNodePtr node,
>  }
>  
>  
> +static virDomainVMGenIDDefPtr
> +virDomainVMGenIDDefParseXML(xmlNodePtr node,
> +                            xmlXPathContextPtr ctxt)
> +{
> +    virDomainVMGenIDDefPtr vmgenid = NULL;
> +    virDomainVMGenIDDefPtr ret = NULL;
> +    xmlNodePtr save = ctxt->node;
> +    char *guidxml = NULL;
> +
> +    ctxt->node = node;
> +
> +    if (VIR_ALLOC(vmgenid) < 0)
> +        goto cleanup;
> +
> +    if (!(guidxml = virXMLPropString(node, "guid"))) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("missing required 'guid' attribute"));
> +        goto cleanup;
> +    }
> +
> +    if (STREQ(guidxml, "auto")) {
> +        vmgenid->autogenerate = true;
> +    } else {
> +        if (virUUIDParse(guidxml, vmgenid->guidstr) < 0) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("malformed guid='%s' provided"), guidxml);
> +            goto cleanup;
> +        }
> +    }
> +
> +    VIR_STEAL_PTR(ret, vmgenid);
> +
> + cleanup:

'guidxml' is leaked

> +    VIR_FREE(vmgenid);
> +    ctxt->node = save;
> +    return ret;
> +}
> +
> +
>  virDomainDeviceDefPtr
>  virDomainDeviceDefParse(const char *xmlStr,
>                          const virDomainDef *def,

[...]

> @@ -21812,6 +21871,25 @@ virDomainIOMMUDefCheckABIStability(virDomainIOMMUDefPtr src,
>  
>  
>  static bool
> +virDomainVMGenIDDefCheckABIStability(virDomainVMGenIDDefPtr src,
> +                                     virDomainVMGenIDDefPtr dst)
> +{
> +    if (memcmp(src->guidstr, dst->guidstr, VIR_UUID_BUFLEN) != 0) {

This does not check 'auto' separately. It will not be able to
differentiate between 'auto' and an UUID of '00000-000...'. Also it will
report weird error message in that case.

Also given that the GUID is to be changed during lifetime of the VM I'm
not sure whether checking it is necessary at all.

> +        char guidsrc[VIR_UUID_STRING_BUFLEN];
> +        char guiddst[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(src->guidstr, guidsrc);
> +        virUUIDFormat(dst->guidstr, guiddst);
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Target domain vmgenid guid '%s' does not match "
> +                         "source '%s'"),
> +                       guiddst, guidsrc);

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux