[libvirt] [PATCH v2 3/5] domain: Document VIR_DOMAIN_XML_MIGRATABLE

Eric Blake posted 5 patches 6 years, 11 months ago
[libvirt] [PATCH v2 3/5] domain: Document VIR_DOMAIN_XML_MIGRATABLE
Posted by Eric Blake 6 years, 11 months ago
Commit 28f8dfdc (1.0.0) added a flag to virDomainGetXMLDesc, but
failed to document its effects.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 src/libvirt-domain.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 54ca18f249..6158382d07 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -2559,7 +2559,13 @@ virDomainGetControlInfo(virDomainPtr domain,
  * currently running domain.  If @flags contains
  * VIR_DOMAIN_XML_UPDATE_CPU, then the portion of the domain XML
  * describing CPU capabilities is modified to match actual
- * capabilities of the host.
+ * capabilities of the host.  If @flags contains VIR_DOMAIN_XML_MIGRATABLE,
+ * the XML is altered to trim redundant information that might interfere
+ * with migration to an older version of libvirt, as well as expose additional
+ * information internal to libvirt; this flag is rejected on read-only
+ * connections, and the resulting XML might not validate against the schema,
+ * but it can serve as a starting point for custom XML in calls such as
+ * virDomainMigrate2().
  *
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
  *         the caller must free() the returned value.
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] domain: Document VIR_DOMAIN_XML_MIGRATABLE
Posted by John Ferlan 6 years, 11 months ago

On 2/14/19 4:29 PM, Eric Blake wrote:
> Commit 28f8dfdc (1.0.0) added a flag to virDomainGetXMLDesc, but
> failed to document its effects.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  src/libvirt-domain.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 54ca18f249..6158382d07 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -2559,7 +2559,13 @@ virDomainGetControlInfo(virDomainPtr domain,
>   * currently running domain.  If @flags contains
>   * VIR_DOMAIN_XML_UPDATE_CPU, then the portion of the domain XML
>   * describing CPU capabilities is modified to match actual
> - * capabilities of the host.
> + * capabilities of the host.  If @flags contains VIR_DOMAIN_XML_MIGRATABLE,
> + * the XML is altered to trim redundant information that might interfere
> + * with migration to an older version of libvirt, as well as expose additional
> + * information internal to libvirt; this flag is rejected on read-only
> + * connections, and the resulting XML might not validate against the schema,
> + * but it can serve as a starting point for custom XML in calls such as
> + * virDomainMigrate2().

You could just use the same attribution from the referenced commit
message, e.g. "virDomainMigrate{,ToURI}2 or virDomainSaveFlags."

I'm not insistent, just following back links. There could be other
examples by now too I suppose.

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

>   *
>   * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
>   *         the caller must free() the returned value.
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] domain: Document VIR_DOMAIN_XML_MIGRATABLE
Posted by Eric Blake 6 years, 11 months ago
On 2/19/19 1:33 PM, John Ferlan wrote:

>> + * capabilities of the host.  If @flags contains VIR_DOMAIN_XML_MIGRATABLE,
>> + * the XML is altered to trim redundant information that might interfere
>> + * with migration to an older version of libvirt, as well as expose additional
>> + * information internal to libvirt; this flag is rejected on read-only
>> + * connections, and the resulting XML might not validate against the schema,
>> + * but it can serve as a starting point for custom XML in calls such as
>> + * virDomainMigrate2().

Not for the public docs, but I may also mention that
VIR_DOMAIN_XML_MIGRATABLE caused CVE-2014-7823 (commit b1674ad5) as part
of my commit message.

> 
> You could just use the same attribution from the referenced commit
> message, e.g. "virDomainMigrate{,ToURI}2 or virDomainSaveFlags."
> 
> I'm not insistent, just following back links. There could be other
> examples by now too I suppose.

Yeah, the latter is what I feared, and did not want to audit for. So I'm
leaving it as written with just the one example.

> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] domain: Document VIR_DOMAIN_XML_MIGRATABLE
Posted by Eric Blake 6 years, 11 months ago
On 2/19/19 1:33 PM, John Ferlan wrote:
> 
> 
> On 2/14/19 4:29 PM, Eric Blake wrote:
>> Commit 28f8dfdc (1.0.0) added a flag to virDomainGetXMLDesc, but
>> failed to document its effects.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  src/libvirt-domain.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index 54ca18f249..6158382d07 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -2559,7 +2559,13 @@ virDomainGetControlInfo(virDomainPtr domain,
>>   * currently running domain.  If @flags contains
>>   * VIR_DOMAIN_XML_UPDATE_CPU, then the portion of the domain XML
>>   * describing CPU capabilities is modified to match actual
>> - * capabilities of the host.
>> + * capabilities of the host.  If @flags contains VIR_DOMAIN_XML_MIGRATABLE,
>> + * the XML is altered to trim redundant information that might interfere
>> + * with migration to an older version of libvirt, as well as expose additional
>> + * information internal to libvirt; this flag is rejected on read-only
>> + * connections, and the resulting XML might not validate against the schema,
>> + * but it can serve as a starting point for custom XML in calls such as
>> + * virDomainMigrate2().
> 
> You could just use the same attribution from the referenced commit
> message, e.g. "virDomainMigrate{,ToURI}2 or virDomainSaveFlags."
> 
> I'm not insistent, just following back links. There could be other
> examples by now too I suppose.

After looking at this one more, I'm going to submit a v3 on this patch
with wording more like:

+ * If @flags contains VIR_DOMAIN_XML_MIGRATABLE, the XML is altered to
+ * assist in migrations where the source and destination are running
+ * different libvirt versions, whether by trimming redundant or
+ * default information that might confuse an older recipient, or by
+ * exposing internal details that aid a newer recipient; this flag is
+ * rejected on read-only connections, and the resulting XML might not
+ * validate against the schema, so it is mainly for internal use.


For the rest of the series, I've made the edits you suggested and
pushed.  Thanks for the review.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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