Re: libvirt-6.5.0 breaks host passthrough migration
On Fri, Jul 10, 2020 at 7:48 AM Mark Mielke wrote:
> On Fri, Jul 10, 2020 at 7:14 AM Jiri Denemark wrote:
>
>> > +if (qemuCaps &&
>>
> > +def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH &&
>> > +!def->cpu->migratable) {
>> > +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_MIGRATABLE))
>> > +def->cpu->migratable = VIR_TRISTATE_SWITCH_ON;
>> >
>> > *+else if (ARCH_IS_X86(def->os.arch))+
>> > def->cpu->migratable = VIR_TRISTATE_SWITCH_OFF;*
>> > +}
>>
>> The implementation seems to be doing exactly what the commit message
>> says. The migratable=off default should be used only when QEMU does not
>> support -cpu host,migratable=on|off, that is only when QEMU is very old.
>> Every non-ancient version of libvirt should have the
>> QEMU_CAPS_CPU_MIGRATABLE set and thus this code should choose
>> migrateble=on default.
>>
>
> I wasn't sure what QEMU_CAPS_CPU_MIGRATABLE represents. I initially
> suspected what you are saying, but since it apparently did not work the way
> I expected, I then presumed it does not work the way I expected. :-)
>
> Is QEMU_CAPS_CPU_MIGRATABLE only from the element? If so, doesn't
> this mean that it is not explicitly listed for host-passthrough, and this
> means the check is not detecting whether it is enabled or not properly?
>
Trying to understand what is going on more - I see "migratable" seems to be
ok when launching a new machine, but the failure scenario was live
migration from 6.4.0 to 6.5.0.
Is this because the QEMU_CAPS_CPU_MIGRATABLE was not filled in for 6.4.0,
and live migration grabs the capabilities from the source, where the
absence of this capability makes it presume an older Qemu in the above code?
--
Mark Mielke
Re: libvirt-6.5.0 breaks host passthrough migration
On Sat, Jul 11, 2020 at 6:04 AM Mark Mielke wrote: > On Fri, Jul 10, 2020 at 7:48 AM Mark Mielke wrote: > >> On Fri, Jul 10, 2020 at 7:14 AM Jiri Denemark >> wrote: >> >>> The implementation seems to be doing exactly what the commit message >>> >> says. The migratable=off default should be used only when QEMU does not >>> support -cpu host,migratable=on|off, that is only when QEMU is very old. >>> Every non-ancient version of libvirt should have the >>> QEMU_CAPS_CPU_MIGRATABLE set and thus this code should choose >>> migrateble=on default. >>> >> QEMU_CAPS_CPU_MIGRATABLE only from the element? If so, doesn't this >> mean that it is not explicitly listed for host-passthrough, and this means >> the check is not detecting whether it is enabled or not properly? >> > Trying to understand what is going on more - I see "migratable" seems to > be ok when launching a new machine, but the failure scenario was live > migration from 6.4.0 to 6.5.0. > > Is this because the QEMU_CAPS_CPU_MIGRATABLE was not filled in for 6.4.0, > and live migration grabs the capabilities from the source, where the > absence of this capability makes it presume an older Qemu in the above code? > Sorry all - I am having trouble reproducing now. The expected use cases are now working. Is it possible that the "migratable" flag might have been missing on some of the instances, although migration worked fine, and despite having used Qemu 4.2 and Qemu 5.0? If I can reproduce it, I'll follow up. Otherwise, thanks for looking. -- Mark Mielke
Re: Setting 'nodatacow' on VM image when on Btrfs
On Fri, Jul 10, 2020 at 6:16 AM Daniel P. Berrangé wrote: > > On Thu, Jul 09, 2020 at 02:15:50PM -0600, Chris Murphy wrote: > > On Thu, Jul 9, 2020 at 12:27 PM Daniel P. Berrangé > > wrote: > > Good point. I am open to suggestion/recommendation by Felipe Borges > > about GNOME Boxes' ability to do installs by injecting kickstarts. It > > might sound nutty but it's quite sane to consider the guest do > > something like plain XFS (no LVM). But all of my VM's are as you > > describe: guest is btrfs with the checksums and compression, on a raw > > file with chattr +C set. > > GNOME Boxes / virt-install both use libosinfo for doing the automated > kickstart installs. In the case of Fedora that's driven by a template > common across all versions. We already have to cope with switch from > ext3 to ext4 way back in Fedora 10. If new Fedora decides on btrfs > by default, we'll need to update the kickstart to follow that > recmmendation too > > https://gitlab.com/libosinfo/osinfo-db/-/blob/master/data/install-script/fedoraproject.org/fedora-kickstart-jeos.xml.in#L80 Understood. > > For lives it's rsync today. I'm not certain if rsync carries over > > file attributes. tar does not. Also not sure if squashfs and > > unsquashfs do either. So this might mean an anaconda post-install > > script is a more reliable way to go, since Anaconda can support rsync > > (Live) and rpm (netinstall,dvd) installs. And there is a proposal > > dangling in the wind to use plain squashfs (no nested ext4 as today). > > Hmm, tricky, so many different scenarios to consider - traditional > Anaconda install, install from live CD, plain RPM post-install, > and pre-built disk image. Also, the location for GNOME Boxes doesn't exist at install time, so the installer doing it with a post-install script isn't a complete solution. While Boxes can use 'qemu-img create -o nocow=on' there is an advantage of 'chattr +C' on the enclosing directory: files copied into it, as well as new files created, inherit the attribute. Meanwhile, it can't be set after the file has non-zero size. > > It seems reasonable to me that libvirtd can "own" > > /var/lib/libvirt/images and make these decisions. i.e. if it's empty, > > and if btrfs, then delete it and recreate as subvolume and also chattr > > +C > > Deleting & recreating as a subvolume is a bit more adventurous than > I would like to be for something done transparently from the user. > I think would need that to be an explicit decision somewhere tied > to the libvirt pool build APIs. Perhaps virt-install/virt-manager > can do this though. It's an optimization, not a prerequisite. And it's reasonable to just cross this bridge if/when we get to it. But it has dual benefits. And also a possible third, which is in a 'btrfs send/receive' use case where the incremental difference of the subvolume can be computed and sent elsewhere, it's possible to separate (e.g. backup policy) sysroot from VM images. 'btrfs send' has a fairly cheap mechanism to identify only changed blocks without deep traversal or comparison. Of course, it has to solve people's actual problems, and prove itself useful. -- Chris Murphy
[PATCH v3] conf: add 'isa' controller type
Decided to revive this patch to address https://gitlab.com/libvirt/libvirt/-/issues/45. Changes from v2: - Rebased to master, - Added bhyveDomainDeviceDefValidate() to disallow ISA controllers idx other than 0 because we don't support more than one ISA controller. Introduce 'isa' controller type. The only supported model now is 'isa-bridge'. In domain XML it looks this way: ... ... Currently, this is needed for the bhyve driver to allow choosing a specific PCI address for that. In bhyve, this controller is used to attach serial ports and a boot ROM. bhyve: support 'isa' controller for LPC Support modeling of the 'isa' controller for bhyve. When controller is not present in the domain XML, but domain requires it to be there (e.g. because bootrom is used), implicitly add addition of this controller to the command line arguments, and bind it to PCI slot 1. PCI slot 1 is always reserved still. User can manually define any PCI slot for the 'isa' controller, including PCI slot 1, but other devices are not allowed to use this address. bhyve: automatically add 'isa' controller When domain configuration requires the 'isa' controller to be present, automatically add it on domain post-parse stage. Now, as this controller is always available when needed, it's not necessary to implicitly add it to the bhyve command line, so remove bhyveBuildLPCArgStr(). Also, make bhyveDomainDefNeedsISAController() static as it's no longer used outside of bhyve_domain.c. bhyve: validate that ISA controller always has index 0 More than one ISA controller is not supported by bhyve, and multiple controllers with the same index are forbidden, so forbid ISA controllers with non-zero index for bhyve. Signed-off-by: Roman Bogorodskiy --- docs/schemas/domaincommon.rng | 6 src/bhyve/bhyve_command.c | 27 +++--- src/bhyve/bhyve_device.c | 23 +--- src/bhyve/bhyve_domain.c | 25 +++-- src/bhyve/bhyve_domain.h | 2 -- src/conf/domain_conf.c| 9 + src/conf/domain_conf.h| 8 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c| 1 + src/qemu/qemu_domain_address.c| 1 + src/qemu/qemu_validate.c | 1 + src/vbox/vbox_common.c| 1 + ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++ ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++ ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++ ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++ ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++ ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++ ...argv-addr-non-isa-controller-on-slot-1.xml | 23 .../bhyvexml2argv-console.args| 2 +- .../bhyvexml2argv-isa-controller.args | 10 ++ .../bhyvexml2argv-isa-controller.ldargs | 3 ++ .../bhyvexml2argv-isa-controller.xml | 24 + ...bhyvexml2argv-isa-multiple-controllers.xml | 25 + .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argv-serial-grub.args| 2 +- .../bhyvexml2argv-serial.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +-- .../bhyvexml2argv-vnc-autoport.args | 4 +-- .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +-- .../bhyvexml2argv-vnc-vgaconf-off.args| 4 +-- .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +-- .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +-- tests/bhyvexml2argvtest.c | 5 +++ ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++ ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++ .../bhyvexml2xmlout-console.xml | 3 ++ .../bhyvexml2xmlout-isa-controller.xml| 36 +++ .../bhyvexml2xmlout-serial-grub-nocons.xml| 3 ++ .../bhyvexml2xmlout-serial-grub.xml | 3 ++ .../bhyvexml2xmlout-serial.xml| 3 ++ .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-io.xml| 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-on.xml| 3 ++ .../bhyvexml2xmlout-vnc.xml | 3 ++ tests/bhyvexml2xmltest.c | 3 ++ 47 files changed, 406 insertions(+), 37 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args create mode 100644 tests/bhyvexm
