[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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 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->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 redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]