On Thu, Aug 06, 2020 at 12:09:20PM +0200, Peter Krempa wrote: > 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. Thank you for the helpful advice and the patches! I'll try to implement the transient disk procedure with the refactor. Thanks! Masa
