Re: [PATCH 0/5 v2] Corrections to SCSI logical unit handling

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

 



On 06/16/2015 11:29 PM, Eric Farman wrote:
> While working with the hostdev tag and SCSI LUNs, a problem was
> discovered with the XML schema (see commit message in patch 4).
> This spawned some further corrections to the handling of the
> logical unit field throughout libvirt.
> 
> This series was split from a single patch, from this feedback:
> http://www.redhat.com/archives/libvir-list/2015-June/msg00489.html
> 
> Eric Farman (5):
>   Print SCSI logical unit as a positive integer
>   Print SCSI logical unit as unsigned integer
>   Convert SCSI logical unit from int to long long
>   docs: Fix XML schema handling of LUN address in hostdev tag
>   docs: Correct typos in scsi hostdev and address elements

I get the value of small patches & agree with the way patches 4 and 5
are split out, but patch 1 and 2 are completely replaced by patch 3 with
a different type.  These are pretty straightforward changes, so I'd
suggest squashing patches 1-3 as a single patch that just goes from
signed int --> unsigned long long and has a commit that explains why we
needed to change both size and sign...  I see now that it was a v1
comment from John, so I'll leave it to his judgment.

Regardless, code still looks good to me so you can feel free to keep my
reviewed-by tag for the set, whether patches 1-3 are squashed or not.

Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxxxxxxx>

> 
>  docs/formatdomain.html.in     | 10 +++++++---
>  docs/schemas/domaincommon.rng | 14 ++++++++++++--
>  src/conf/domain_audit.c       |  2 +-
>  src/conf/domain_conf.c        |  4 ++--
>  src/conf/domain_conf.h        |  2 +-
>  src/qemu/qemu_command.h       |  2 +-
>  src/qemu/qemu_hotplug.c       |  4 ++--
>  src/util/virhostdev.c         |  6 +++---
>  src/util/virscsi.c            | 16 ++++++++--------
>  src/util/virscsi.h            |  8 ++++----
>  tests/testutilsqemu.c         |  2 +-
>  tools/virsh-domain.c          |  6 +++---
>  12 files changed, 45 insertions(+), 31 deletions(-)
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]