[libvirt] [PATCH 2/2] qemu: add usb-serial support

[Eric Blake]

On 01/02/2013 11:57 PM, Guannan Ren wrote:
> Add an optional 'type' attribute to <target> element of serial port
> device. There are two choices for its value, 'isa-serial' and
> 'usb-serial'. For backward compatibility, when attribute 'type' is
> missing the 'isa-serial' will be chose as before.

s/chose/chosen/

> 
> Libvirt XML sample
> 
>     <serial type='pty'>
>       <target type='usb-serial' port='0'/>
>       <address type='usb' bus='0' port='1'/>
>     </serial>
> 
> qemu commandline:
> 
> qemu ${other_vm_args}              \
>     -chardev pty,id=charserial0    \
>     -device usb-serial,chardev=charserial0,id=serial0,bus=usb.0,port=1

> +++ b/docs/formatdomain.html.in
> @@ -3677,7 +3677,14 @@ qemu-kvm -net nic,model=? /dev/null
>      <p>
>        <code>target</code> can have a <code>port</code> attribute, which
>        specifies the port number. Ports are numbered starting from 0. There are
> -      usually 0, 1 or 2 serial ports.
> +      usually 0, 1 or 2 serial ports. There is also an optional
> +      <code>type</code> attribute <span class="since">Since 1.0.2</span>

This is mid-sentence, so s/Since/since/

> +      which has two choices for its value, one is< code>isa-serial</code>,
> +      the other is <code>usb-serial</code>. If <code>type</code> is missing,
> +      <code>isa-serial</code> will be used by default. For <code>usb-serial</code>
> +      an optional sub-element <code><address></code> with
> +      <code>type='usb'</code>which can tie the device to a particular controller,
> +      <a href="#elementsAddress">documentedabove</a>.

s/documentedabove/documented above/

> @@ -994,6 +1001,8 @@ struct _virDomainChrSourceDef {
>  /* A complete character device, both host and domain views.  */
>  struct _virDomainChrDef {
>      int deviceType;
> +
> +    bool targetType_attr;

The naming looks funny, when you are mixing camelCase and
underscore_split in the same name.  Also, do we really need this
boolean?  How many test files would have to change if we change all
existing outputs to always include type='isa-serial', where the field is
optional only on input; vs. your approach of using this bool to omit the
field on output if the user did not specify it on input?

Overall it looks reasonable, but depending on the answer to whether we
should just always output type='isa-serial', you may have a v2 to submit.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

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