[libvirt PATCH 1/5] conf: define a new "maximum" CPU mode

[Pavel Hrdina]

On Wed, Feb 10, 2021 at 11:01:24AM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 10, 2021 at 11:53:53AM +0100, Pavel Hrdina wrote:
> > On Wed, Feb 10, 2021 at 10:39:22AM +0000, Daniel P. Berrangé wrote:
> > > On Tue, Feb 09, 2021 at 03:30:47PM +0100, Pavel Hrdina wrote:
> > > > On Tue, Feb 09, 2021 at 01:58:57PM +0000, Daniel P. Berrangé wrote:
> > > > > For hardware virtualization this is functionally identical to the
> > > > > existing host-passthrough mode so the same caveats apply.
> > > > > 
> > > > > For emulated guest this exposes the maximum featureset supported by
> > > > > the emulator. Note that despite being emulated this is not guaranteed
> > > > > to be migration safe, especially if different emulator software versions
> > > > > are used on each host.
> > > > > 
> > > > > Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> > > > > ---
> > > > >  docs/formatdomain.rst        | 24 ++++++++++++++++++++++++
> > > > >  src/conf/cpu_conf.c          | 13 +++++++++----
> > > > >  src/conf/cpu_conf.h          |  1 +
> > > > >  src/cpu/cpu.c                |  1 +
> > > > >  src/qemu/qemu_capabilities.c |  1 +
> > > > >  src/qemu/qemu_command.c      |  1 +
> > > > >  src/qemu/qemu_domain.c       |  1 +
> > > > >  src/qemu/qemu_validate.c     |  1 +
> > > > >  8 files changed, 39 insertions(+), 4 deletions(-)
> > > > 
> > > > [...]
> > > > 
> > > > > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> > > > > index f98b0a0eb3..eb4bfbbcfa 100644
> > > > > --- a/src/conf/cpu_conf.c
> > > > > +++ b/src/conf/cpu_conf.c
> > > > > @@ -402,10 +403,11 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
> > > > >      if ((migratable = virXMLPropString(ctxt->node, "migratable"))) {
> > > > >          int val;
> > > > >  
> > > > > -        if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
> > > > > +        if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH &&
> > > > > +            def->mode != VIR_CPU_MODE_MAXIMUM) {
> > > > >              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > > > >                             _("Attribute migratable is only allowed for "
> > > > > -                             "host-passthrough CPU"));
> > > > > +                             "'host-passthrough' / 'maximum' CPU mode"));
> > > > 
> > > > Since you are modifying the error message I would reformat it to single
> > > > line.
> > > 
> > > That'd would make the line waaaaay longer than normal practice. I know
> > > we don't need to strictly stick to 80 chars, but at the same time we
> > > don't want things massively long.
> > 
> > I'm aware of that. We did not change all errors and we probably don't
> > have a syntax-check for that but there was an agreement while ago that
> > for error messages we can ignore the 80 chars limit [1].
> 
> Within reason I think that's ok, but I think this example gets a bit
> too long at nearly 120 chars

I figured that we don't care about the line being that long. The main
point about not splitting error messages is to help with git grep.

> > 
> > Feel free to push it like it is, I just wanted to point out that and
> > personally I try to change errors to single line if I have to modify
> > them
> > 
> > Pavel
> > 
> > [1] <https://libvirt.org/coding-style.html#error-message-format>
> 
> The example here was already under 80 chars even when on one
> line.

We should definitely improve the example to make it more obvious.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210210/ec262b41/attachment-0001.sig>