[libvirt] [PATCH libvirt-virshcmdref] Add blockpull and blockjob commands

[Stefan Hajnoczi]

On Wed, Aug 22, 2012 at 12:07:51PM -0600, Eric Blake wrote:
> On 08/22/2012 07:40 AM, Stefan Hajnoczi wrote:
> > +++ b/source/blockjob.xml
> > +    <parameter requirement="required">
> > +      <value type="string" requirement="required">path</value>
> > +      <description>
> > +        The fully-qualified path of the disk.  See "domblklist" for listing these names.
> 
> Maybe mention that you can give either the path (/path/to/disk) or the
> devname (vda).

Good idea.

> > +    <parameter requirement="optional">
> > +      <keyword requirement="required">--async</keyword>
> > +      <description>
> > +        Return immediately instead of waiting for cancelation to complete, when specified together with "--abort".
> 
> s/cancelation/cancellation/

Okay.

> > +      </description>
> > +    </parameter>
> > +    <parameter requirement="optional">
> > +      <keyword requirement="required">--pivot</keyword>
> > +    </parameter>
> 
> What a shame that qemu 1.2 still doesn't support block copy, and
> therefore --pivot is still a no-op (it only makes sense when ending a
> copy job).

I was thinking whether to drop --pivot completely from this page.  I
also haven't attempted to document blockcopy since I've never used the
command and mirroring isn't upstream in QEMU.

> > +    <parameter requirement="optional">
> > +      <keyword requirement="required">--info</keyword>
> > +      <description>
> > +        Print information about any active block operation.
> > +      </description>
> > +    </parameter>
> > +  </options>
> > +
> > +  <availability from="0.9.4" />
> 
> Should we start listing which version of virsh added various options?
> For example, --pivot wasn't present until 0.9.12 (commit 1f06c00), but
> still has no backend that supports it (except RHEL 6.3, via
> RHEL-specific patches).

I'm not aware of XML for per-option version info and plan to leave it at
0.9.4 for now.

> > +++ b/source/blockpull.xml
> 
> > +    <parameter requirement="required">
> > +      <value type="string" requirement="required">path</value>
> > +      <description>
> > +        The fully-qualified path of the disk.  See "domblklist" for listing these names.
> 
> Same story about accepting full path or devname.
> 
> 
> > +    <example>
> > +      <terminal>virsh # <bold>blockpull</bold> <value>example-domain</value> <value>vda</value> <value>0</value> <value>/path/to/backing.img</value></terminal>
> > +      <text>
> > +        Start populating <value>vda</value> from its backing image chain up to <value>/path/to/backing.img</value> and return immediately.  <value>/path/to/backing.img</value> and its backing images will not be flattened.  Note that the <value>0</value> means unlimited bandwidth and is necessary because <value>bandwidth</value> and <value>base</value> are positional arguments.
> 
> Long line (here and elsewhere, but this one stood out to me).  Can you
> please wrap things to fit in 80 columns?

Sure.

> Your comment is not quite true; this is an equivalent command line that
> omits the bandwidth:
> 
> blockpull example-domain vda --base /path/to/backing.img
> 
> by instead using an explicit '--base'.

Thanks, --base is nicer.  I'll drop the '0'.

Stefan