Re: [libvirt] [PATCH] docs: formatdomain: Specify the default value of 'check' attribute

On Tue, Jan 09, 2018 at 19:44:18 +0100, Kashyap Chamarthy wrote: > On Tue, Jan 09, 2018 at 04:37:10PM +0100, Jiri Denemark wrote: > > On Tue, Jan 09, 2018 at 15:32:54 +0100, Kashyap Chamarthy wrote: > > [...] > > > > But doesn't tell *what* the default value is. It is check='partial'. > > > Mention it so. > > [...] > > > NACK > > > > As I said on IRC, the default differs with guest architecture ('partial' > > for x86_64, 'none' for all others). > > Yep, I missed to distinguish that detail. > > > Moreover this only applies to QEMU driver. Oh and it even changes > > based on the mode attribute to make not make it easy. But this is > > really an implementation detail. Libvirt will just use a suitable > > default to stay compatible with older releases. > > That's all good. If you think nothing needs to be clarified here, > that's OK. (Some admins who operate a lot of VMs with libvirt will > disagree with you, though.) I'm not saying there's nothing to be clarified. Just that explicitly specifying the default value will not help at all since it's complicated and confusing by itself especially because of backward compatibility. > And as we saw on IRC, `virt-install` muddies the water in this case by > gratuitously adding a CPU parameter with check='full' (even if a user > didn't explicitly specify it) when you simply import an image and look > at the run-time guest XML: > > $> virt-install --name vm1 --ram 2048 \ > --disk path=./vm1.qcow2,format=qcow2 --nographics \ > --import --os-variant fedora27 > > $> virsh dumpxml vm1 | grep check > <cpu mode='custom' match='exact' check='full'> > > (You might, fairly, argue here that: "Well, that's a bug in > `virt-install`, go complain there.") I guess it's just the default behavior of virt-install and it automatically passes the host CPU model to domain XML (without check attribute, relying on the default) when creating new domains. After all, any CPU model is going to be better than the default qemu64.

> It all comes across as messy to an unsuspecting admin. (If a competent > admin finds it confusing, good luck to new admins who want to wrap their > head around this.) > > - - - > > Background for others reading: The admin who reported this was confused > when he was creating guests with `virt-install`, which adds check='full' > (as noted earlier), and the guest throws: > > error: Failed to start domain foo.org > error: operation failed: guest CPU doesn't match specification: extra features: vme,arat If this really happens when creating a domain with virt-install, then it's clearly a bug. But I don't believe it's what happened. The dumpxml above says check='full', which means the domain was successfully started and its CPU def was updated according to QEMU and thus the check attribute was changed to 'full'. The vme and arat features are added because libvirt's and QEMU's vision of the particual CPU model differs. Libvirt specifies it without vme and arat while QEMU has them both included in the CPU model. Thus when libvirt asks for that CPU model, QEMU enables these extra features too and libvirt adds them to the domain XML so that it can make sure they don't disappear when the domain is migrated or save/restored. > So "somehow" QEMU added the CPU features 'vme' and 'arat' by itself, now > you have to specify them in libvirt. So the admin ended up with a `sed` > one-liner that updates the guest XML with the missing features: > > sed -i -e "s-</cpu>-<feature policy='require' name='vme'/></cpu>-" > sed -i -e "s-</cpu>-<feature policy='require' name='arat'/></cpu>-" This is some strange mangling of the XML by the admin for unclear reason. It would be nice to finally see what the admin wanted to achieve, what steps they did, and what result they saw.