Re: [Xen-devel] [RFC V8 2/3] libxl domain snapshot API design

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-12-08 at 22:04 -0700, Chun Yan Liu wrote:
> Partly. At least for domain disk snapshot create/delete, I prefer using
> qmp commands instead of calling qemu-img one by one. Using qmp
> commands, libvirt will need libxl's help. Of course, if libxl doesn't
> supply that, libvirt can call qemu-img to each disk one by one,
> not preferred but can do.

You can't use qmp unless the domain is active, for an inactive domain
there is no qemu to talk to, so you have to use qemu-img anyway in that
case. Does libvirt not have existing code to do all this sort of thing?
(I thought so, but it turns out I may be wrong, see below).

And for an active domain I expect that *must* use qmp, since it seems
unlikely that you can go around changing things under the feet of an
active process (maybe I'm wrong?).

> > > Following the constraint that it's better NOT to supply disk snapshot 
> > > functions in libxl, then we let xl and libvirt do that by themselve 
> > > separately, that's OK. 
> > >  
> > > Then I think NO new API needs to be exported in libxl, since: 
> > > * saving/restoring memory, there are already APIs. 
> >  
> > The principle is that if existing API doesn't work good enough for you 
> > we will consider adding a new one. 
> >  
> > We probably need a new API. If you want to do a live snapshot, we would 
> > need to notify xl that we are in the middle of pausing and resuming a 
> > domain.  
> 
> This is where we discussed a lot. Do we really need
> libxl_domain_snapshot_create API? or does xl can do the work?
> 
> Even for live snapshot, after calling libxl_domain_suspend with LIVE flags,
> memory is saved and domain is paused. xl then can call disk snapshot
> functions to finish disk snapshots, after all of that, call 
> libxl_domain_unpause
> to unpause the domain. So I don't think xl has any trouble to do that.
> In case there is some misunderstanding, please point out.

My mistake, I incorrectly remembered that libxl_domain_suspend would
destroy (for save or migate) or resume (for checkpoint) the guest before
returning. Having refreshed my memory I see that you are correct: it
returns with the domain paused and it is up to the toolstack to resume
or destroy it as it wishes. Sorry for the confusion.

Given that it does seem like the toolstack could indeed take the
disksnapshots itself without an additional API.

> > However the current architecture for libvirt to use libxl is like 
> >  
> >   libvirt 
> >   libxl 
> >   [other lower level stuffs] 
> >  
> > So implementing snapshot management in xl cannot work for you either. 
> > It's not part of the current architecture.

This is correct, xl should not be involved in a libvirt control stack,
it is orthogonal.

> You are right. I understand you are trying to suggest a way to ease the job.
> Here just to make clear this way is really better and finally acceptable? :-)
> Just IMO, I think xl snapshot-list is wanted, that means managing snapshots
> in xl is needed.

The xl idiom is that you do this sort of operation with existing CLI
commands e.g. ls /var/lib/vm-images/*.qcow2, lvs, qemu-img etc.

Adding snapshot-list to xl would be a whole load of work to create a
bunch of infrastructure which you do not need to do.

My understanding was that your primary aim here was to enable snapshots
via libvirt and that xl support was just a nice to have. Is that right?

> > Not that I'm against the idea of managing domain snapshot in xl, I'm 
> > trying to reduce workload here. 
> >  
> > > > The  
> > > > downside is that now xl and libvirt are disconnected, but I think it's  
> > > > fine. 
> > >  
> > > Two things here: 
> > > 1. connect xl and libvirt, then will need to manage snapshot info in 
> > > libxl  
> > (or 
> > >     libxlu) That's not preferred since the initial design. This is not 
> > > the  
> > point 
> > >     we discuss here. 
> > > 2. for xl only, list snapshots and delete snapshots, also need to manage 
> > >     snapshot info (in xl) 
> > >  
> > > Considering manage snapshot info in xl, only question is about idl and 
> > > gentypes.py, expected structure is as following and expected to be saved 
> > > into json file, but it contains xl namespace and libxl namespace things, 
> > > gentypes.py will have problem. Better ideas? 
> > >  
> > > typedef struct xl_domain_snapshot { 
> > >     char * name; 
> > >     char * description; 
> > >     uint64_t creation_time; 
> > >     char * memory_path; 
> > >     int num_disks; 
> > >     libxl_disk_snapshot *disks; 
> > >     char *parent; 
> > >     bool *current; 
> > > } xl_domain_snapshot; 
> > >  
> >  
> > The libxl_disk_snapshot suggests that you still want storage management 
> > inside libxl, which should probably be in libxlu? 
> 
> Yeah. I may put it in libxlu.

This depends on who the consumers of this datastructure are:

If xl only -> put it in xl itself.
If libvirt+xl -> put it in libxlu.

My understanding was that libvirt already has data structures for
dealing with snapshots, but this was based entirely on the commands
listed by:
        virsh help | grep -E pool-\|snapshot-
which seemed to me to be pretty feature rich and suggested that libvirt
has a great deal of support for storage and snapshot management already.

If libvirt already has generic infrastructure for managing snapshots
this then IMHO you should use it, not reimplement it on the Xen side
(whether in libxl, libxlu or xl), the additions to Xen should be limited
to providing the underlying functionality which libvirt's generic code
requires from the backend.

However, Wei has suggested to me that perhaps libvirt's snapshotting
capabilities are not as generic internally as I might have imaged and
that it is up to each backend driver to reinvent things, is that true?

If Wei's suggestion is correct then it may turn out that it is useful to
put some of the new generic code which you would need to write into
libxlu.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel