[libvirt] [PATCH v5 5/9] pvs: get info about serial ports

[Eric Blake]

On 04/20/2012 10:01 AM, Dmitry Guryanov wrote:
> Add support of collecting information about serial
> ports. This change is needed mostly as an example,
> support of other devices will be added later.
> 
> Signed-off-by: Dmitry Guryanov <dguryanov at parallels.com>
> ---
>  src/pvs/pvs_driver.c |  115 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 115 insertions(+), 0 deletions(-)

This is as far as I got today; at this point, it's probably okay for you
to start respinning your patches (some of my comments have been pretty
common between the patches).


>  
> +static int
> +pvsGetSerialInfo(virDomainChrDefPtr chr,
> +                 const char *name, virJSONValuePtr value)
> +{
> +    const char *tmp;
> +
> +    chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL;
> +    chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
> +    chr->target.port = atoi(name + strlen("serial"));

atoi() is a no-no, because it fails to detect errors (but we don't yet
enable the syntax-check rule for it; I should really fix that some day).
 virStrToLong_i() is your friend.

Overall, this series is looking relatively decent (it's compiling and
mostly passing syntax checks, other than the po/POTFILES.in issues).
I'm not really in a position to test it, but I'm okay accepting it for
0.9.12, since as new code it can't really be causing regressions.  I'm
not sure if it will be in before DV cuts rc1, but I think it's probably
okay even if we miss the first release candidate build before you can
get my review comments incorporated.

-- 
Eric Blake   eblake at 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: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120501/8b43cfe3/attachment-0001.sig>