[libvirt] [PATCH v3] blockjob: use stable disk string in job event

[Peter Krempa]

On 06/19/14 00:41, Eric Blake wrote:
> When the block job event was first added, it was for block pull,
> where the active layer of the disk remains the same name.  It was
> also in a day where we only cared about local files, and so we
> always had a canonical absolute file name.  But two things have
> changed since then: we now have network disks, where determining
> a single absolute string does not really make sense; and we have
> two-phase jobs (copy and active commit) where the name of the
> active layer changes between the first event (ready, on the old
> name) and second (complete, on the pivoted name).
> 
> Adam Litke reported that having an unstable string between events
> makes life harder for clients.  Furthermore, all of our API that
> operate on a particular disk of a domain accept multiple strings:
> not only the absolute name of the active layer, but also the
> destination device name (such as 'vda').  As this latter name is
> stable, even for network sources, it serves as a better string
> to supply in block job events.
> 
> But backwards-compatibility demands that we should not change the
> name handed to users unless they explicitly request it.  Therefore,
> this patch adds a new event, BLOCK_JOB_2 (alas, I couldn't think of
> any nicer name - but at least Migrate2 and Migrate3 are precedent
> for a number suffix).  We must double up on emitting both old-style
> and new-style events according to what clients have registered for
> (see also how IOError and IOErrorReason emits double events, but
> there the difference was a larger struct rather than changed
> meaning of one of the struct members).
> 
> Unfortunately, adding a new event isn't something that can easily
> be broken into pieces, so the commit is rather large.
> 
> * include/libvirt/libvirt.h.in (virDomainEventID): Add a new id
> for VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2.
> (virConnectDomainEventBlockJobCallback): Document new semantics.
> * src/conf/domain_event.c (_virDomainEventBlockJob): Rename field,
> to ensure we catch all clients.
> (virDomainEventBlockJobNew): Add parameter.
> (virDomainEventBlockJobDispose)
> (virDomainEventBlockJobNewFromObj)
> (virDomainEventBlockJobNewFromDom)
> (virDomainEventDispatchDefaultFunc): Adjust clients.
> (virDomainEventBlockJob2NewFromObj)
> (virDomainEventBlockJob2NewFromDom): New functions.
> * src/conf/domain_event.h: Add new prototypes.
> * src/libvirt_private.syms (domain_event.h): Export new functions.
> * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Generate two
> different events.
> * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Likewise.
> * src/remote/remote_protocol.x
> (remote_domain_event_block_job_2_msg): New struct.
> (REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2): New RPC.
> * src/remote/remote_driver.c
> (remoteDomainBuildEventBlockJob2): New handler.
> (remoteEvents): Register new event.
> * daemon/remote.c (remoteRelayDomainEventBlockJob2): New handler.
> (domainEventCallbacks): Register new event.
> * tools/virsh-domain.c (vshEventCallbacks): Likewise.
> (vshEventBlockJobPrint): Adjust client.
> * src/remote_protocol-structs: Regenerate.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> 
> v3: don't do s/path/disk/ on code that is not shared; s/disk/dst/ on
> new code that is not shared; touch up libvirt.c docs, actually test
> with python bindings
> 
>  daemon/remote.c              | 39 +++++++++++++++++++++++++++++++++++++++
>  include/libvirt/libvirt.h.in | 19 ++++++++++++++++---
>  src/conf/domain_event.c      | 44 +++++++++++++++++++++++++++++++++++---------
>  src/conf/domain_event.h      | 11 +++++++++++
>  src/libvirt_private.syms     |  2 ++
>  src/qemu/qemu_driver.c       |  8 +++++++-
>  src/qemu/qemu_process.c      |  7 +++++++
>  src/remote/remote_driver.c   | 31 +++++++++++++++++++++++++++++++
>  src/remote/remote_protocol.x | 16 +++++++++++++++-
>  src/remote_protocol-structs  |  8 ++++++++
>  tools/virsh-domain.c         |  7 +++++--
>  11 files changed, 176 insertions(+), 16 deletions(-)
> 

I almost didn't notice this one as I tend to skip cover letters and this
looked just like one.

ACK,

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140619/4bdda28a/attachment-0001.sig>