[libvirt] Selective block device migration implementation

[Pavel Boldin]

On Wed, Apr 15, 2015 at 6:43 PM, Michal Privoznik <mprivozn at redhat.com>
wrote:

> On 15.04.2015 16:38, Pavel Boldin wrote:
> > Michal,
> >
> > On Wed, Apr 15, 2015 at 10:54 AM, Michal Privoznik <mprivozn at redhat.com>
> > wrote:
> >
> >> On 26.03.2015 15:48, Pavel Boldin wrote:
> >>> Dear Libvirt Developers,
> >>>
> >>>
> >>> I'm working to implement feature request [1]. The feature request
> >> proposes
> >>> to enhance `libvirt' code so the API caller can specify which block
> >> devices
> >>> are to be migrated using e.g. parameters in the
> `virDomainMigrateToURI3'
> >>> call.
> >>
> >> Correct. My idea is to add pairs of typed parameters to the
> >> virDomainMigrate3() and virDomainMigrateToURI3() API. These are the only
> >> migrate APIs that accept virTypedParameter. The rest of migrate APIs are
> >> just too old and not extensible, so they must continue working as they
> >> are now. However, those two APIs - they can be passed new pairs, e.g.
> >> ("migrate_disk", "vda"). Now, mgmt applications will want to specify
> >> more than one disk to migrate, obviously. We have two options:
> >>
> >> 1) make "migrate_disk" accept stringyfied array of disk targets, e.g.
> >> "migrate_disk": "vda,vdb,sda". This is not so pleasant to work with from
> >> client applications. Constructing the string would be awful.
> >>
> > My concern was that some drivers may allow commas inside the drive names.
> > So this is obviously wrong thing to do then.
>
> That's why I've chosen to work purely with disk target at NBD level. We
> have strong rules what characters can occur there. Moreover, it's fairly
> easy to derive qemu disk ID from the target. Oh, and we require targets
> to be unique throughout the domain. So I think it's the best option for
> the extension you're planning.
>
As far as I remember, libvirt-1.2.2 does not support tunneled migration
through NBD.


>
> >
> >
> >>
> >> 2) make virTypedParam* APIs accept more than one values to a key, e.g.
> >> {("migrate_disk", "vda"), ("migrate_disk", "vdb"), ("migrate_disk",
> >> "sda")}. Currently, this is supported on RPC, but some virTypedParam*
> >> APIs are not really prepared for this. If I had to name some from the
> >> top of my head: virTypedParamsGet().
> >>
> > As far as I see there is a line 420 in src/util/virtypedparam.c:
> > /* The following APIs are public and their signature may never change. */
>
> Of course. They would still return the first value found. But that's not
> a problem. The paragraph is more like an intro than a suggestion to
> change them.
>
> >
> > So, the functions need to be implemented anew.
>
> Some of them. Correct.
>
> >
> >
> >
> >>
> >> I view virTyped* as an associative array. So 2) is practically enabling
> >> multi value associative array. So I guess the first step would be to
> >> introduce new API (maybe set of APIs?) that are aware of this.
> >>
> > I guess the first function to implement would be virTypedParamsGetNth()
> > that is able to retrieve nth parameter with that name.
> > While this is easy to implement there are details remain:
> >
> >    1. Should we implement a new set of virTypedParamsGetNth<TYPE> for
> each
> >    of the TYPE or should we start with just the 'String'?
> >    2. Since virTypedParamsGet<TYPE> is just a virTypedParamsGetNth<TYPE>
> >    with N=0 should we make old functions just call new ones with N=0?
> >
>
> Hm.. what about this, introduce just this new function:
>
> virTypedParamsGetArrayForKey(.., const char *key, ...)
>
> which will iterate through an array of typed params, producing a new
> array which will, however, contain only the selected key. For instance,
> if called over
>
> {(key1, val1), (key1, val2), (key2, val3), (key4, val4)}
>
> with key==key1 it would create array
>
> {(key1, val1), (key1, val2)}
>
> with key==key2 it would create array
>
> {(key2, val3)}
>
> And so on. For selecting Nth item in the array, we can just use basic C
> array selector []. Oh, I don't like the function name much btw, but it
> serves the example sake here.
>
Well, this approach allows us to return virTypedParamPtr array of any type.
So this is more general and the latter can be implemented as a call to this
one.


>
> The other option would be to have a different function:
>
> char **
> virTypedParamGetStringList( ..., const char *key)
>
> which will produce a string list of values assigned with the @key. The
> list will be NULL terminated of course. Again, the function name is
> horrible, but it's just an example.
>
> In fact, the latter approach would be much more easier to use, so I vote
> for it.
>
Yeah, and it is much easier to add an extra parameter `char **migrate_disk'
to all the QEMU functions stack.

I think I will implement both, second one calling first one.


>
> Oh, and these functions I came up with - they don't even need to be
> public! We can keep them private and live happily ever after. Well, not
> quite - the virTypedParamsValidate() will require some changes too, but
> whatever.
>
We will need to add a `virTypedParamsValidateDups' function or something
like that.

I'm going to start looking into the code now. Hopefully, I will have an API
implementation till the end of the week.

No comments below this line.

>
> >
> >
> >>
> >> Then, virDomainMigrate*3() can just use this in the following way:
> >>
> >> a) when no "migrate_disk" is specified, the current behaviour is
> preserved
> >>
> >> b) when there are some disk selected for storage migration, since at
> >> this point we have virTyped* APIs to work with multivalue, we can get
> >> the array of disk targets to migrate and instruct qemu to just migrate
> >> them.
> >>
> >> Now, qemu has split storage and internal state migration into two
> >> streams. The first one is called NBD and libvirt selectively choses
> >> disks to migrate. For the other stream you don't have to care about.
> >> Look at qemuMigrationDriveMirror() and you'll see how libvirt instructs
> >> qemu to selectively migrate only some disks. The "migrate_disk" array
> >> would need to be propagated down here then.
> >>
> > My concern is I would, most likely, have to backport these to our
> versions.
>
> I'm not sure onto how old release, but in general, the smaller the patch
> is, the easier to backport it is also ;-)
>
> Michal
>

Pavel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150415/ec24f221/attachment-0001.htm>