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

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(a)redhat.com&gt; > > > > --- > > > > 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

> > 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.