Re: [PATCH 1/3] qemu: implementation of transient option for qcow2 file

On Sun, Jul 19, 2020 at 22:56:50 -0400, Masayoshi Mizuma wrote: > On Sat, Jul 18, 2020 at 08:06:00AM +0200, Peter Krempa wrote: > > On Thu, Jul 16, 2020 at 20:55:29 -0400, Masayoshi Mizuma wrote: > > > Thank you for your review. > > > > > > On Tue, Jul 07, 2020 at 06:36:23AM -0500, Eric Blake wrote: > > > > On 7/7/20 2:12 AM, Peter Krempa wrote: > > > > > > > > > You can install a qcow2 overlay on top of a raw file too. IMO the > > > > > implications of using <transient/> allow that. > > > > > > > > > > As said above I'd strongly prefer if the overlay is created in qemu > > > > > using the blockdev-create blockjob (there is already infrastructure in > > > > > libvirt to achieve that). > > > > > > > > Agreed. At this point, any time we call out to qemu-img as a separate > > > > process, we are probably doing it wrong. > > > > > > Got it. I'm thinking about the procedure such as followings. > > > Does that make sense? > > > > > > 1) Open the monitor with qemuProcessQMPNew()/qemuProcessQMPStart(), > > > and connect it. > > > > Starting a new qemu process just to format an image is extreme overkill > > and definitely not what we want to do. > > I see, thanks. > > > > > > 2) Setup the transient disk with qemuDomainPrepareStorageSourceBlockdev(), > > > qemuBlockStorageSourceAttachApplyStorage(), qemuBlockStorageSourceCreateGetFormatProps() > > > and something... > > > > > > 3) Run blockdev-create command with qemuMonitorBlockdevCreate(), then > > > close the monitor. > > > > These two steps should be exectued in the qemu process which already > > will run the VM prior to starting the guest CPUs. > > > > > 4) Switch the original disk to the transient disk. > > > > > > 5) Build the blockdev argument for qemu. > > > > And instead of this step, you use the external snapshot infrastructure > > to install the overlays via 'blockdev-snapshot' QMP command > > OK. I suppose qemuDomainSnapshotDiskPrepare() and > qemuDomainSnapshotDiskUpdateSource() maybe helpful to implement the > setup steps of transient disk. > > > > > > > > > 6) Run qemu > > > > And instead of this the VM cpus will be started. > > Got it, I think the appropriate place is after virCommandRun() at qemuProcessLaunch(), > and before qemuProcessFinishStartup(). > > > > > > > The above steps require factoring out snapshot code a bit. I have a few > > patches in that direction so I'll try posting them next week hopefully. Sorry this took longer than expected, but we were dealing with the build system change. The refactor is here: https://www.redhat.com/archives/libvir-list/2020-August/msg00299.html You can now create an equivalent of 'qemuSnapshotDiskPrepare' which will go through the disks and find the 'transient' ones. It will then create snapshot data by a call to 'qemuSnapshotDiskPrepareOne' with a faked snapshot disk object. 'qemuSnapshotDiskPrepareOne' ensures that the files are created and formatted properly. You then use same algorithm as 'qemuSnapshotCreateDiskActive' (e.g. by extracting the common internals (basically everything except call to 'qemuSnapshotDiskPrepare') into a separate function) and reuse it when starting the VM as you've described above. Note that all of the above can work only when QEMU_CAPS_BLOCKDEV is supported. The caveats/limitations with blockjobs and snapshots still apply though. It depends on how you approach it. It's okay to limit/block the features if transient disk is used. Alternatively you can implement some form of private data to mark which image was transient and allow those operations.