
On Wed, 2015-06-17 at 10:29 -0400, Matthew Rosato wrote:
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@linux.vnet.ibm.com> I'm with you. Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
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(-)