[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]