Michal,

On Wed, Apr 15, 2015 at 10:54 AM, Michal Privoznik <mprivozn@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.

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. */

So, the functions need to be implemented anew.

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:

Should we implement a new set of virTypedParamsGetNth<TYPE> for each of the TYPE or should we start with just the 'String'?
Since virTypedParamsGet<TYPE> is just a virTypedParamsGetNth<TYPE> with N=0 should we make old functions just call new ones with N=0?

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.

Pavel

Hopefully I haven't forgot nothing.

Michal