[libvirt] [PATCH RFC 0/5] drop nested job concept

[Peter Krempa]

On Mon, Nov 28, 2016 at 11:15:41 +0300, Nikolay Shirokovskiy wrote:
> Instead I suggest to serialize qemu monitor commands.

That won't be possible. There are a few APIs that call multiple commands
and expect the VM not to change in between. Thus they rely on nested
jobs. See migration and snapshot code (btw, the two most complex parts
of libvirt).

> My original motivation is to prepare job exclusion code to add agent jobs. This
> job is a special job to operate on guest agent only so first it can be run in
> parallel with sync, async and nested jobs and second it should not block any

That is possible. The async jobs allow for query jobs to be run while
they are executed you need to properly configure them.

> qemu monitor job at all. Eventually I want to move from a lot of different
> types of jobs to just a job with some kind of matrix which describes what jobs
> can run in a parallel. As a first step I want to drop nested jobs.

I don't think you will be able to do this in steps. Nested jobs have a
vital functions and unless you replace the mechanism or overhaul all the
code that is doing nested jobs you won't be able to guarantee that the
state of the objects are as the code expects.

Doin a ful N*N matrix of possible parallel operations will be very hard
to maintain. You will need to group them together in a way so that we
don't have to deal with the quadratic complexity. How are you expecting
to do this?

> AFAIU *main* nested job purpuse is to serialize requests to monitor from async
> jobs and allowed sync jobs as monitor does not do it by himself now. Strictly

No, the main purpose is to allow to call multiple monitor commands while
you are guaranteed that something did not change the VM state and thus
invalidate the complex operation you are trying to execute.

> speaking this assertion is rather rough because nested jobs and sync jobs
> serialized at job level and thus chunks of code that serialized differ from
> case when only monitor requests are serialized. Nevertheless IFAIU this part of
> libvirt is not strict too. We have a lot of query commands and async commands
> that can execute in parallel and have no definition of what query job or async
> job are. Thus it is not clear is this code correct or not. So the same argument

Certainly it's more correct this way than dropping it without
replacement.

> is applicable to new approach: query jobs are safe to be called during async
> jobs.  (Except that new approach opens situation when async job interrupts

Some of qemu monitor commands block on execution. The job mechanism
prevents the second query call to get stuck waiting forever for the job
to finish by doing a timed wait.

> query job itself but it generally not clear how this can break query job).
> On this path job became high level concept which I think it should be.  Also we
> drop boring passing of current async job which we need only for safety check
> deep down below the stack.  We drop driver passing for nested jobs which
> I guess we really don't need to pass at all as we need it only to check queue
> size and nested job probably should skip this check.

I did not get what you wanted to say here.

> The diff stat looks large but most of code is dumb renaming or dropping unused
> parameter in patches 4 and 5. To reduce risk of introducing an error on this
> tedious task I used sed and perl where I can.
> 
> I ran only a simple test with this series of quering a domain during its
> migration.

That is a weak test for doing such a complex change. I'd suggest you run
parallel migrations and snapshot to see if it works. You need to run
operations that change the VM configuration.

I don't think we can do this with a complete overhaul of snapshots,
migration and a few others that actually have an expectation of the VM
object not changing while we are in an async job.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161128/9012a2ee/attachment-0001.sig>