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

Re: [libvirt] [PATCH v2 10/21] conf: Parse and format virDomainChrSerialTargetModel



On Tue, Nov 21, 2017 at 05:42:20PM +0100, Andrea Bolognani wrote:
> This information will be used to select, and store in the guest
> configuration in order to guarantee ABI stability, the concrete
> (hypervisor-specific) model for serial devices.
> 
> Signed-off-by: Andrea Bolognani <abologna redhat com>
> ---
>  docs/schemas/domaincommon.rng | 15 +++++++++
>  src/conf/domain_conf.c        | 72 ++++++++++++++++++++++++++++++++++++++++++-
>  src/conf/domain_conf.h        | 12 ++++++++
>  3 files changed, 98 insertions(+), 1 deletion(-)

This patch doesn't include documentation for the new element, it's in
the followup patch 22/21 which is not ideal.  I guess that we will need
v3 :).  It's common to add the documentation together with the patch
that introduces new change to the domain XML so I would suggest to
split the patch 22/21 into the part that cleanups and improves the
current documentation without mentioning new features and the <model>
documentation should be part of this patch to make it close together.

Otherwise this patch looks good.

> 
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f1808065b..fbba092d1 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3589,6 +3589,18 @@
>      </attribute>
>    </define>
>  
> +  <define name='qemucdevSerialTgtModel'>

This name is horrible and ugly, but I see that it follows naming for
other defines.  I know that nobody will send a patch to change it, but
I wanted to point it out :).

Pavel

Attachment: signature.asc
Description: PGP signature


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