10:29 p.m.
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
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(-)
--
1.9.1
10:29 p.m.
New subject: [libvirt] [PATCH 1/5] Print SCSI logical unit as a positive integer
A logical unit address is a positive integer, so it would be wise
to reject any negative inputs.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 2 +-
tools/virsh-domain.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca55981..9e77b87 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4954,7 +4954,7 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
goto cleanup;
}
- if (virStrToLong_ui(unit, NULL, 0, &scsihostsrc->unit) < 0) {
+ if (virStrToLong_uip(unit, NULL, 0, &scsihostsrc->unit) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse unit '%s'"), unit);
goto cleanup;
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4c47473..0bea462 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -488,7 +488,7 @@ static int str2SCSIAddress(const char *str, struct SCSIAddress
*scsiAddr)
return -1;
unit++;
- if (virStrToLong_ui(unit, NULL, 0, &scsiAddr->unit) != 0)
+ if (virStrToLong_uip(unit, NULL, 0, &scsiAddr->unit) != 0)
return -1;
return 0;
--
1.9.1
11:47 a.m.
New subject: [libvirt] [PATCH 1/5] Print SCSI logical unit as a positive integer
On 06/16/2015 11:29 PM, Eric Farman wrote:
A logical unit address is a positive integer, so it would be wise
to reject any negative inputs.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 2 +-
tools/virsh-domain.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
While we're at it - why not bus and target as well? That's doable in
one patch... I'll adjust this in order
Looking back through history it seems commit id '5c811dce' added this
functionality to domain.conf.c and commit id 'e962a579' added address
parsing for the attach-disk command.
I also peeked at the review cycle of the domain_conf.c change and found
my name, but at the time I didn't notice that we were expecting a non
negative value storing as unsigned, but printing as signed... In my
defense I was newer to the group though ;-)
So, I'll add bus/target as _uip and then adjust the title/commit message
appropriately when I push the changes.
John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca55981..9e77b87 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4954,7 +4954,7 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
goto cleanup;
}
- if (virStrToLong_ui(unit, NULL, 0, &scsihostsrc->unit) < 0) {
+ if (virStrToLong_uip(unit, NULL, 0, &scsihostsrc->unit) < 0)
{
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse unit '%s'"),
unit);
goto cleanup;
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4c47473..0bea462 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -488,7 +488,7 @@ static int str2SCSIAddress(const char *str, struct SCSIAddress
*scsiAddr)
return -1;
unit++;
- if (virStrToLong_ui(unit, NULL, 0, &scsiAddr->unit) != 0)
+ if (virStrToLong_uip(unit, NULL, 0, &scsiAddr->unit) != 0)
return -1;
return 0;
1:01 p.m.
New subject: [libvirt] [PATCH 1/5] Print SCSI logical unit as a positive integer
On 06/18/2015 12:47 PM, John Ferlan wrote:
On 06/16/2015 11:29 PM, Eric Farman wrote:
> A logical unit address is a positive integer, so it would be wise
> to reject any negative inputs.
>
> Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
> ---
> src/conf/domain_conf.c | 2 +-
> tools/virsh-domain.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
While we're at it - why not bus and target as well? That's doable in
one patch... I'll adjust this in order
The linux kernel/qemu code have some more confusion in how the bus and
target are printed, but that's certainly not the responsibility of this
patch series. The LUN is always printed as %llu in the other layers of
the stack, so I just wanted to get them in sync.
So yes, I'm fine with cleaning up bus and target in both these patches.
Thanks!
- Eric
Looking back through history it seems commit id '5c811dce' added this
functionality to domain.conf.c and commit id 'e962a579' added address
parsing for the attach-disk command.
I also peeked at the review cycle of the domain_conf.c change and found
my name, but at the time I didn't notice that we were expecting a non
negative value storing as unsigned, but printing as signed... In my
defense I was newer to the group though ;-)
So, I'll add bus/target as _uip and then adjust the title/commit message
appropriately when I push the changes.
John
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ca55981..9e77b87 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4954,7 +4954,7 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr
sourcenode,
> goto cleanup;
> }
>
> - if (virStrToLong_ui(unit, NULL, 0, &scsihostsrc->unit) <
0) {
> + if (virStrToLong_uip(unit, NULL, 0, &scsihostsrc->unit) <
0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("cannot parse unit '%s'"),
unit);
> goto cleanup;
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 4c47473..0bea462 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -488,7 +488,7 @@ static int str2SCSIAddress(const char *str, struct SCSIAddress
*scsiAddr)
> return -1;
>
> unit++;
> - if (virStrToLong_ui(unit, NULL, 0, &scsiAddr->unit) != 0)
> + if (virStrToLong_uip(unit, NULL, 0, &scsiAddr->unit) != 0)
> return -1;
>
> return 0;
>
10:29 p.m.
New subject: [libvirt] [PATCH 2/5] Print SCSI logical unit as unsigned integer
The logical unit field is an unsigned integer, we should
use the appropriate substitution when printing it.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
---
src/conf/domain_audit.c | 2 +-
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_hotplug.c | 4 ++--
src/util/virhostdev.c | 6 +++---
src/util/virscsi.c | 6 +++---
tools/virsh-domain.c | 2 +-
6 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 1900039..c94cae8 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -427,7 +427,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr
hostdev,
} else {
virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
&scsisrc->u.host;
- if (virAsprintfQuiet(&address, "%s:%d:%d:%d",
+ if (virAsprintfQuiet(&address, "%s:%d:%d:%u",
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target,
scsihostsrc->unit) < 0) {
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9e77b87..7e3ca36 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18940,7 +18940,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
virBufferAsprintf(buf, "<adapter name='%s'/>\n",
scsihostsrc->adapter);
virBufferAsprintf(buf,
- "<address %sbus='%d' target='%d'
unit='%d'/>\n",
+ "<address %sbus='%d' target='%d'
unit='%u'/>\n",
includeTypeInAddr ? "type='scsi' " :
"",
scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index cc86a3b..1d538a0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1938,7 +1938,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
} else {
virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to prepare scsi hostdev: %s:%d:%d:%d"),
+ _("Unable to prepare scsi hostdev: %s:%d:%d:%u"),
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit);
}
@@ -3873,7 +3873,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
&scsisrc->u.host;
virReportError(VIR_ERR_OPERATION_FAILED,
- _("host scsi device %s:%d:%d.%d not found"),
+ _("host scsi device %s:%d:%d.%u not found"),
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit);
}
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 1c8f31e..ea0076c 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1482,7 +1482,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr hostdev_mgr,
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit,
hostdev->readonly, hostdev->shareable))) {
- VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%d on domain %s",
+ VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%u on domain %s",
scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit, dom_name);
return;
@@ -1492,7 +1492,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr hostdev_mgr,
* because qemuProcessStart could fail half way through. */
if (!(tmp = virSCSIDeviceListFind(hostdev_mgr->activeSCSIHostdevs, scsi))) {
- VIR_WARN("Unable to find device %s:%d:%d:%d "
+ VIR_WARN("Unable to find device %s:%d:%d:%u "
"in list of active SCSI devices",
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit);
@@ -1500,7 +1500,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr hostdev_mgr,
return;
}
- VIR_DEBUG("Removing %s:%d:%d:%d dom=%s from activeSCSIHostdevs",
+ VIR_DEBUG("Removing %s:%d:%d:%u dom=%s from activeSCSIHostdevs",
scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit, dom_name);
diff --git a/src/util/virscsi.c b/src/util/virscsi.c
index 9f5cf0d..6c8b6ce 100644
--- a/src/util/virscsi.c
+++ b/src/util/virscsi.c
@@ -123,7 +123,7 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix,
return NULL;
if (virAsprintf(&path,
- "%s/%d:%d:%d:%d/scsi_generic",
+ "%s/%d:%d:%d:%u/scsi_generic",
prefix, adapter_id, bus, target, unit) < 0)
return NULL;
@@ -170,7 +170,7 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix,
return NULL;
if (virAsprintf(&path,
- "%s/%d:%d:%d:%d/block",
+ "%s/%d:%d:%d:%u/block",
prefix, adapter_id, bus, target, unit) < 0)
return NULL;
@@ -227,7 +227,7 @@ virSCSIDeviceNew(const char *sysfs_prefix,
if (virSCSIDeviceGetAdapterId(adapter, &dev->adapter) < 0)
goto cleanup;
- if (virAsprintf(&dev->name, "%d:%d:%d:%d", dev->adapter,
+ if (virAsprintf(&dev->name, "%d:%d:%d:%u", dev->adapter,
dev->bus, dev->target, dev->unit) < 0 ||
virAsprintf(&dev->sg_path, "%s/%s",
sysfs_prefix ? sysfs_prefix : "/dev", sg) < 0)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0bea462..e9dbcd9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -725,7 +725,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (diskAddr.type == DISK_ADDR_TYPE_SCSI) {
virBufferAsprintf(&buf,
"<address type='drive'
controller='%d'"
- " bus='%d' unit='%d'
/>\n",
+ " bus='%d' unit='%u'
/>\n",
diskAddr.addr.scsi.controller, diskAddr.addr.scsi.bus,
diskAddr.addr.scsi.unit);
} else {
--
1.9.1
12:05 p.m.
New subject: [libvirt] [PATCH 2/5] Print SCSI logical unit as unsigned integer
On 06/16/2015 11:29 PM, Eric Farman wrote:
The logical unit field is an unsigned integer, we should
use the appropriate substitution when printing it.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
---
src/conf/domain_audit.c | 2 +-
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_hotplug.c | 4 ++--
src/util/virhostdev.c | 6 +++---
src/util/virscsi.c | 6 +++---
tools/virsh-domain.c | 2 +-
6 files changed, 11 insertions(+), 11 deletions(-)
Similar to 1/5 - why only adjust unit, adjust bus & target too since
they're incorrect. I will adjust when I push (and change commit message
to reflect that).
John
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 1900039..c94cae8 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -427,7 +427,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr
hostdev,
} else {
virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
&scsisrc->u.host;
- if (virAsprintfQuiet(&address, "%s:%d:%d:%d",
+ if (virAsprintfQuiet(&address, "%s:%d:%d:%u",
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target,
scsihostsrc->unit) < 0) {
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9e77b87..7e3ca36 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18940,7 +18940,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
virBufferAsprintf(buf, "<adapter name='%s'/>\n",
scsihostsrc->adapter);
virBufferAsprintf(buf,
- "<address %sbus='%d' target='%d'
unit='%d'/>\n",
+ "<address %sbus='%d' target='%d'
unit='%u'/>\n",
includeTypeInAddr ? "type='scsi' " :
"",
scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index cc86a3b..1d538a0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1938,7 +1938,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
} else {
virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to prepare scsi hostdev: %s:%d:%d:%d"),
+ _("Unable to prepare scsi hostdev: %s:%d:%d:%u"),
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit);
}
@@ -3873,7 +3873,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
&scsisrc->u.host;
virReportError(VIR_ERR_OPERATION_FAILED,
- _("host scsi device %s:%d:%d.%d not found"),
+ _("host scsi device %s:%d:%d.%u not found"),
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit);
}
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 1c8f31e..ea0076c 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1482,7 +1482,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr
hostdev_mgr,
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit,
hostdev->readonly, hostdev->shareable))) {
- VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%d on domain %s",
+ VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%u on domain %s",
scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit, dom_name);
return;
@@ -1492,7 +1492,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr
hostdev_mgr,
* because qemuProcessStart could fail half way through. */
if (!(tmp = virSCSIDeviceListFind(hostdev_mgr->activeSCSIHostdevs, scsi))) {
- VIR_WARN("Unable to find device %s:%d:%d:%d "
+ VIR_WARN("Unable to find device %s:%d:%d:%u "
"in list of active SCSI devices",
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit);
@@ -1500,7 +1500,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr
hostdev_mgr,
return;
}
- VIR_DEBUG("Removing %s:%d:%d:%d dom=%s from activeSCSIHostdevs",
+ VIR_DEBUG("Removing %s:%d:%d:%u dom=%s from activeSCSIHostdevs",
scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit, dom_name);
diff --git a/src/util/virscsi.c b/src/util/virscsi.c
index 9f5cf0d..6c8b6ce 100644
--- a/src/util/virscsi.c
+++ b/src/util/virscsi.c
@@ -123,7 +123,7 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix,
return NULL;
if (virAsprintf(&path,
- "%s/%d:%d:%d:%d/scsi_generic",
+ "%s/%d:%d:%d:%u/scsi_generic",
prefix, adapter_id, bus, target, unit) < 0)
return NULL;
@@ -170,7 +170,7 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix,
return NULL;
if (virAsprintf(&path,
- "%s/%d:%d:%d:%d/block",
+ "%s/%d:%d:%d:%u/block",
prefix, adapter_id, bus, target, unit) < 0)
return NULL;
@@ -227,7 +227,7 @@ virSCSIDeviceNew(const char *sysfs_prefix,
if (virSCSIDeviceGetAdapterId(adapter, &dev->adapter) < 0)
goto cleanup;
- if (virAsprintf(&dev->name, "%d:%d:%d:%d", dev->adapter,
+ if (virAsprintf(&dev->name, "%d:%d:%d:%u", dev->adapter,
dev->bus, dev->target, dev->unit) < 0 ||
virAsprintf(&dev->sg_path, "%s/%s",
sysfs_prefix ? sysfs_prefix : "/dev", sg) < 0)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0bea462..e9dbcd9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -725,7 +725,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (diskAddr.type == DISK_ADDR_TYPE_SCSI) {
virBufferAsprintf(&buf,
"<address type='drive'
controller='%d'"
- " bus='%d' unit='%d'
/>\n",
+ " bus='%d' unit='%u'
/>\n",
diskAddr.addr.scsi.controller,
diskAddr.addr.scsi.bus,
diskAddr.addr.scsi.unit);
} else {
10:29 p.m.
New subject: [libvirt] [PATCH 3/5] Convert SCSI logical unit from int to long long
The SCSI Architecture Model defines a logical unit address
as 64-bits in length, so change the field accordingly so
that the entire value could be stored.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
---
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 +++---
10 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index c94cae8..e5d7e15 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -427,7 +427,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr
hostdev,
} else {
virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
&scsisrc->u.host;
- if (virAsprintfQuiet(&address, "%s:%d:%d:%u",
+ if (virAsprintfQuiet(&address, "%s:%d:%d:%llu",
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target,
scsihostsrc->unit) < 0) {
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7e3ca36..3f0f175 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4954,7 +4954,7 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
goto cleanup;
}
- if (virStrToLong_uip(unit, NULL, 0, &scsihostsrc->unit) < 0) {
+ if (virStrToLong_ullp(unit, NULL, 0, &scsihostsrc->unit) < 0)
{
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse unit '%s'"), unit);
goto cleanup;
@@ -18940,7 +18940,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
virBufferAsprintf(buf, "<adapter name='%s'/>\n",
scsihostsrc->adapter);
virBufferAsprintf(buf,
- "<address %sbus='%d' target='%d'
unit='%u'/>\n",
+ "<address %sbus='%d' target='%d'
unit='%llu'/>\n",
includeTypeInAddr ? "type='scsi' " :
"",
scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ba17a8d..f677c2e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -463,7 +463,7 @@ struct _virDomainHostdevSubsysSCSIHost {
char *adapter;
unsigned bus;
unsigned target;
- unsigned unit;
+ unsigned long long unit;
};
typedef struct _virDomainHostdevSubsysSCSIiSCSI virDomainHostdevSubsysSCSIiSCSI;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 0fc59a8..6e0c3a3 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -63,7 +63,7 @@ struct _qemuBuildCommandLineCallbacks {
const char *adapter,
unsigned int bus,
unsigned int target,
- unsigned int unit);
+ unsigned long long unit);
};
extern qemuBuildCommandLineCallbacks buildCommandLineCallbacks;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1d538a0..d5a40aa 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1938,7 +1938,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
} else {
virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to prepare scsi hostdev: %s:%d:%d:%u"),
+ _("Unable to prepare scsi hostdev: %s:%d:%d:%llu"),
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit);
}
@@ -3873,7 +3873,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
&scsisrc->u.host;
virReportError(VIR_ERR_OPERATION_FAILED,
- _("host scsi device %s:%d:%d.%u not found"),
+ _("host scsi device %s:%d:%d.%llu not found"),
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit);
}
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index ea0076c..aa06b4a 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1482,7 +1482,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr hostdev_mgr,
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit,
hostdev->readonly, hostdev->shareable))) {
- VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%u on domain %s",
+ VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%llu on domain %s",
scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit, dom_name);
return;
@@ -1492,7 +1492,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr hostdev_mgr,
* because qemuProcessStart could fail half way through. */
if (!(tmp = virSCSIDeviceListFind(hostdev_mgr->activeSCSIHostdevs, scsi))) {
- VIR_WARN("Unable to find device %s:%d:%d:%u "
+ VIR_WARN("Unable to find device %s:%d:%d:%llu "
"in list of active SCSI devices",
scsihostsrc->adapter, scsihostsrc->bus,
scsihostsrc->target, scsihostsrc->unit);
@@ -1500,7 +1500,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr hostdev_mgr,
return;
}
- VIR_DEBUG("Removing %s:%d:%d:%u dom=%s from activeSCSIHostdevs",
+ VIR_DEBUG("Removing %s:%d:%d:%llu dom=%s from activeSCSIHostdevs",
scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit, dom_name);
diff --git a/src/util/virscsi.c b/src/util/virscsi.c
index 6c8b6ce..ea4a738 100644
--- a/src/util/virscsi.c
+++ b/src/util/virscsi.c
@@ -56,7 +56,7 @@ struct _virSCSIDevice {
unsigned int adapter;
unsigned int bus;
unsigned int target;
- unsigned int unit;
+ unsigned long long unit;
char *name; /* adapter:bus:target:unit */
char *id; /* model:vendor */
@@ -110,7 +110,7 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix,
const char *adapter,
unsigned int bus,
unsigned int target,
- unsigned int unit)
+ unsigned long long unit)
{
DIR *dir = NULL;
struct dirent *entry;
@@ -123,7 +123,7 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix,
return NULL;
if (virAsprintf(&path,
- "%s/%d:%d:%d:%u/scsi_generic",
+ "%s/%d:%d:%d:%llu/scsi_generic",
prefix, adapter_id, bus, target, unit) < 0)
return NULL;
@@ -157,7 +157,7 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix,
const char *adapter,
unsigned int bus,
unsigned int target,
- unsigned int unit)
+ unsigned long long unit)
{
DIR *dir = NULL;
struct dirent *entry;
@@ -170,7 +170,7 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix,
return NULL;
if (virAsprintf(&path,
- "%s/%d:%d:%d:%u/block",
+ "%s/%d:%d:%d:%llu/block",
prefix, adapter_id, bus, target, unit) < 0)
return NULL;
@@ -200,7 +200,7 @@ virSCSIDeviceNew(const char *sysfs_prefix,
const char *adapter,
unsigned int bus,
unsigned int target,
- unsigned int unit,
+ unsigned long long unit,
bool readonly,
bool shareable)
{
@@ -227,7 +227,7 @@ virSCSIDeviceNew(const char *sysfs_prefix,
if (virSCSIDeviceGetAdapterId(adapter, &dev->adapter) < 0)
goto cleanup;
- if (virAsprintf(&dev->name, "%d:%d:%d:%u", dev->adapter,
+ if (virAsprintf(&dev->name, "%d:%d:%d:%llu", dev->adapter,
dev->bus, dev->target, dev->unit) < 0 ||
virAsprintf(&dev->sg_path, "%s/%s",
sysfs_prefix ? sysfs_prefix : "/dev", sg) < 0)
@@ -347,7 +347,7 @@ virSCSIDeviceGetTarget(virSCSIDevicePtr dev)
return dev->target;
}
-unsigned int
+unsigned long long
virSCSIDeviceGetUnit(virSCSIDevicePtr dev)
{
return dev->unit;
diff --git a/src/util/virscsi.h b/src/util/virscsi.h
index c67837f..df40d7f 100644
--- a/src/util/virscsi.h
+++ b/src/util/virscsi.h
@@ -37,18 +37,18 @@ char *virSCSIDeviceGetSgName(const char *sysfs_prefix,
const char *adapter,
unsigned int bus,
unsigned int target,
- unsigned int unit);
+ unsigned long long unit);
char *virSCSIDeviceGetDevName(const char *sysfs_prefix,
const char *adapter,
unsigned int bus,
unsigned int target,
- unsigned int unit);
+ unsigned long long unit);
virSCSIDevicePtr virSCSIDeviceNew(const char *sysfs_prefix,
const char *adapter,
unsigned int bus,
unsigned int target,
- unsigned int unit,
+ unsigned long long unit,
bool readonly,
bool shareable);
@@ -61,7 +61,7 @@ const char *virSCSIDeviceGetName(virSCSIDevicePtr dev);
unsigned int virSCSIDeviceGetAdapter(virSCSIDevicePtr dev);
unsigned int virSCSIDeviceGetBus(virSCSIDevicePtr dev);
unsigned int virSCSIDeviceGetTarget(virSCSIDevicePtr dev);
-unsigned int virSCSIDeviceGetUnit(virSCSIDevicePtr dev);
+unsigned long long virSCSIDeviceGetUnit(virSCSIDevicePtr dev);
bool virSCSIDeviceGetReadonly(virSCSIDevicePtr dev);
bool virSCSIDeviceGetShareable(virSCSIDevicePtr dev);
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index d067bca..ceaabb6 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -462,7 +462,7 @@ testSCSIDeviceGetSgName(const char *sysfs_prefix ATTRIBUTE_UNUSED,
const char *adapter ATTRIBUTE_UNUSED,
unsigned int bus ATTRIBUTE_UNUSED,
unsigned int target ATTRIBUTE_UNUSED,
- unsigned int unit ATTRIBUTE_UNUSED)
+ unsigned long long unit ATTRIBUTE_UNUSED)
{
char *sg = NULL;
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e9dbcd9..a4620bf 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -415,7 +415,7 @@ struct PCIAddress {
struct SCSIAddress {
unsigned int controller;
unsigned int bus;
- unsigned int unit;
+ unsigned long long unit;
};
struct IDEAddress {
@@ -488,7 +488,7 @@ static int str2SCSIAddress(const char *str, struct SCSIAddress
*scsiAddr)
return -1;
unit++;
- if (virStrToLong_uip(unit, NULL, 0, &scsiAddr->unit) != 0)
+ if (virStrToLong_ullp(unit, NULL, 0, &scsiAddr->unit) != 0)
return -1;
return 0;
@@ -725,7 +725,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (diskAddr.type == DISK_ADDR_TYPE_SCSI) {
virBufferAsprintf(&buf,
"<address type='drive'
controller='%d'"
- " bus='%d' unit='%u'
/>\n",
+ " bus='%d' unit='%llu'
/>\n",
diskAddr.addr.scsi.controller, diskAddr.addr.scsi.bus,
diskAddr.addr.scsi.unit);
} else {
--
1.9.1
1:32 p.m.
New subject: [libvirt] [PATCH 3/5] Convert SCSI logical unit from int to long long
On 06/16/2015 11:29 PM, Eric Farman wrote:
The SCSI Architecture Model defines a logical unit address
as 64-bits in length, so change the field accordingly so
that the entire value could be stored.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
---
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 +++---
10 files changed, 26 insertions(+), 26 deletions(-)
ACK -
John
10:29 p.m.
New subject: [libvirt] [PATCH 4/5] docs: Fix XML schema handling of LUN address in hostdev tag
Defining a domain with a SCSI disk attached via a hostdev
tag and a source address unit value longer than two digits
causes an error when editing the domain with virsh edit,
even if no changes are made to the domain definition.
The error suggests invalid XML, somewhere:
# virsh edit lmb_guest
error: XML document failed to validate against schema:
Unable to validate doc against /usr/local/share/libvirt/schemas/domain.rng
Extra element devices in interleave
Element domain failed to validate content
The virt-xml-validate tool fails with a similar error:
# virt-xml-validate lmb_guest.xml
Relax-NG validity error : Extra element devices in interleave
lmb_guest.xml:17: element devices: Relax-NG validity error :
Element domain failed to validate content
lmb_guest.xml fails to validate
The hostdev tag requires a source address to be specified,
which includes bus, target, and unit address attributes.
According to the SCSI Architecture Model spec (section
4.9 of SAM-2), a LUN address is 64 bits and thus could be
up to 20 decimal digits long. Unfortunately, the XML
schema limits this string to just two digits. Similarly,
the target field can be up to 32 bits in length, which
would be 10 decimal digits.
# lsscsi -xx
[0:0:19:0x4022401100000000] disk IBM 2107900 3.44 /dev/sda
# lsscsi
[0:0:19:1074872354]disk IBM 2107900 3.44 /dev/sda
# cat lmb_guest.xml
<domain type='kvm'>
<name>lmb_guest</name>
<memory unit='MiB'>1024</memory>
...trimmed...
<devices>
<controller type='scsi' model='virtio-scsi'
index='0'/>
<hostdev mode='subsystem' type='scsi'>
<source>
<adapter name='scsi_host0'/>
<address bus='0' target='19' unit='1074872354'/>
</source>
</hostdev>
...trimmed...
Since the reference unit and target fields are used in
several places in the XML schema, create a separate one
specific for SCSI Logical Units that will permit the
greater length. This permits both the validation utility
and the virsh edit command to succeed when a hostdev
tag is included.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
docs/formatdomain.html.in | 6 +++++-
docs/schemas/domaincommon.rng | 14 ++++++++++++--
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4e85b51..c88c4a6 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3256,7 +3256,11 @@
</dd>
<dt>scsi</dt>
<dd>SCSI devices are described by both the
<code>adapter</code>
- and <code>address</code> elements.
+ and <code>address</code> elements. The
<code>address</code>
+ element includes a <code>bus</code> attribute (a 2-digit bus
+ number), a <code>target</code> attribute (a 10-digit target
+ number), and a <code>unit</code> attribute (a 20-digit unit
+ number on the bus).
<p>
<span class="since">Since 1.2.8</span>, the
<code>source</code>
element of a SCSI device may contain the <code>protocol</code>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f0f7400..b3c5cb8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3833,10 +3833,10 @@
<ref name="driveBus"/>
</attribute>
<attribute name="target">
- <ref name="driveTarget"/>
+ <ref name="driveSCSITarget"/>
</attribute>
<attribute name="unit">
- <ref name="driveUnit"/>
+ <ref name="driveSCSIUnit"/>
</attribute>
</define>
<define name="usbportaddress">
@@ -5129,11 +5129,21 @@
<param name="pattern">[0-9]{1,2}</param>
</data>
</define>
+ <define name="driveSCSITarget">
+ <data type="string">
+ <param name="pattern">[0-9]{1,10}</param>
+ </data>
+ </define>
<define name="driveUnit">
<data type="string">
<param name="pattern">[0-9]{1,2}</param>
</data>
</define>
+ <define name="driveSCSIUnit">
+ <data type="string">
+ <param name="pattern">[0-9]{1,20}</param>
+ </data>
+ </define>
<define name="featureName">
<data type="string">
<param name='pattern'>[a-zA-Z0-9\-_\.]+</param>
--
1.9.1
1:38 p.m.
New subject: [libvirt] [PATCH 4/5] docs: Fix XML schema handling of LUN address in hostdev tag
On 06/16/2015 11:29 PM, Eric Farman wrote:
Defining a domain with a SCSI disk attached via a hostdev
tag and a source address unit value longer than two digits
causes an error when editing the domain with virsh edit,
even if no changes are made to the domain definition.
The error suggests invalid XML, somewhere:
# virsh edit lmb_guest
error: XML document failed to validate against schema:
Unable to validate doc against /usr/local/share/libvirt/schemas/domain.rng
Extra element devices in interleave
Element domain failed to validate content
The virt-xml-validate tool fails with a similar error:
# virt-xml-validate lmb_guest.xml
Relax-NG validity error : Extra element devices in interleave
lmb_guest.xml:17: element devices: Relax-NG validity error :
Element domain failed to validate content
lmb_guest.xml fails to validate
The hostdev tag requires a source address to be specified,
which includes bus, target, and unit address attributes.
According to the SCSI Architecture Model spec (section
4.9 of SAM-2), a LUN address is 64 bits and thus could be
up to 20 decimal digits long. Unfortunately, the XML
schema limits this string to just two digits. Similarly,
the target field can be up to 32 bits in length, which
would be 10 decimal digits.
# lsscsi -xx
[0:0:19:0x4022401100000000] disk IBM 2107900 3.44 /dev/sda
# lsscsi
[0:0:19:1074872354]disk IBM 2107900 3.44 /dev/sda
# cat lmb_guest.xml
<domain type='kvm'>
<name>lmb_guest</name>
<memory unit='MiB'>1024</memory>
...trimmed...
<devices>
<controller type='scsi' model='virtio-scsi'
index='0'/>
<hostdev mode='subsystem' type='scsi'>
<source>
<adapter name='scsi_host0'/>
<address bus='0' target='19' unit='1074872354'/>
</source>
</hostdev>
...trimmed...
Since the reference unit and target fields are used in
several places in the XML schema, create a separate one
specific for SCSI Logical Units that will permit the
greater length. This permits both the validation utility
and the virsh edit command to succeed when a hostdev
tag is included.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
docs/formatdomain.html.in | 6 +++++-
docs/schemas/domaincommon.rng | 14 ++++++++++++--
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4e85b51..c88c4a6 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3256,7 +3256,11 @@
</dd>
<dt>scsi</dt>
<dd>SCSI devices are described by both the
<code>adapter</code>
- and <code>address</code> elements.
+ and <code>address</code> elements. The
<code>address</code>
+ element includes a <code>bus</code> attribute (a 2-digit bus
+ number), a <code>target</code> attribute (a 10-digit target
+ number), and a <code>unit</code> attribute (a 20-digit unit
+ number on the bus).
Since we know from the v1 comments that qemu has a max of 16384 units, add:
Not all hypervisors support larger <code>unit</code> values. It is up to
each hypervisor to determine the maximum value supported for the adapter.
I could add "<code>target</code> and " if you think that would be
better..
<p>
<span class="since">Since 1.2.8</span>, the
<code>source</code>
element of a SCSI device may contain the <code>protocol</code>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f0f7400..b3c5cb8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3833,10 +3833,10 @@
<ref name="driveBus"/>
</attribute>
<attribute name="target">
- <ref name="driveTarget"/>
+ <ref name="driveSCSITarget"/>
</attribute>
<attribute name="unit">
- <ref name="driveUnit"/>
+ <ref name="driveSCSIUnit"/>
</attribute>
</define>
<define name="usbportaddress">
@@ -5129,11 +5129,21 @@
<param name="pattern">[0-9]{1,2}</param>
</data>
</define>
+ <define name="driveSCSITarget">
+ <data type="string">
+ <param name="pattern">[0-9]{1,10}</param>
+ </data>
+ </define>
<define name="driveUnit">
<data type="string">
<param name="pattern">[0-9]{1,2}</param>
</data>
</define>
+ <define name="driveSCSIUnit">
+ <data type="string">
+ <param name="pattern">[0-9]{1,20}</param>
+ </data>
+ </define>
<define name="featureName">
<data type="string">
<param name='pattern'>[a-zA-Z0-9\-_\.]+</param>
I'm going to add a test as well. It's essentially a copy of
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-scsi.xml
adjusted to have a larger unit value (it'll be attached)
3:05 p.m.
New subject: [libvirt] [PATCH 4/5] docs: Fix XML schema handling of LUN address in hostdev tag
On 06/18/2015 02:38 PM, John Ferlan wrote:
On 06/16/2015 11:29 PM, Eric Farman wrote:
> Defining a domain with a SCSI disk attached via a hostdev
> tag and a source address unit value longer than two digits
> causes an error when editing the domain with virsh edit,
> even if no changes are made to the domain definition.
> The error suggests invalid XML, somewhere:
>
> # virsh edit lmb_guest
> error: XML document failed to validate against schema:
> Unable to validate doc against /usr/local/share/libvirt/schemas/domain.rng
> Extra element devices in interleave
> Element domain failed to validate content
>
> The virt-xml-validate tool fails with a similar error:
>
> # virt-xml-validate lmb_guest.xml
> Relax-NG validity error : Extra element devices in interleave
> lmb_guest.xml:17: element devices: Relax-NG validity error :
> Element domain failed to validate content
> lmb_guest.xml fails to validate
>
> The hostdev tag requires a source address to be specified,
> which includes bus, target, and unit address attributes.
> According to the SCSI Architecture Model spec (section
> 4.9 of SAM-2), a LUN address is 64 bits and thus could be
> up to 20 decimal digits long. Unfortunately, the XML
> schema limits this string to just two digits. Similarly,
> the target field can be up to 32 bits in length, which
> would be 10 decimal digits.
>
> # lsscsi -xx
> [0:0:19:0x4022401100000000] disk IBM 2107900 3.44 /dev/sda
> # lsscsi
> [0:0:19:1074872354]disk IBM 2107900 3.44 /dev/sda
> # cat lmb_guest.xml
> <domain type='kvm'>
> <name>lmb_guest</name>
> <memory unit='MiB'>1024</memory>
> ...trimmed...
> <devices>
> <controller type='scsi' model='virtio-scsi'
index='0'/>
> <hostdev mode='subsystem' type='scsi'>
> <source>
> <adapter name='scsi_host0'/>
> <address bus='0' target='19'
unit='1074872354'/>
> </source>
> </hostdev>
> ...trimmed...
>
> Since the reference unit and target fields are used in
> several places in the XML schema, create a separate one
> specific for SCSI Logical Units that will permit the
> greater length. This permits both the validation utility
> and the virsh edit command to succeed when a hostdev
> tag is included.
>
> Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
> Reviewed-by: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com>
> Reviewed-by: Stefan Zimmermann <stzi(a)linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
> ---
> docs/formatdomain.html.in | 6 +++++-
> docs/schemas/domaincommon.rng | 14 ++++++++++++--
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 4e85b51..c88c4a6 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3256,7 +3256,11 @@
> </dd>
> <dt>scsi</dt>
> <dd>SCSI devices are described by both the
<code>adapter</code>
> - and <code>address</code> elements.
> + and <code>address</code> elements. The
<code>address</code>
> + element includes a <code>bus</code> attribute (a 2-digit
bus
> + number), a <code>target</code> attribute (a 10-digit target
> + number), and a <code>unit</code> attribute (a 20-digit unit
> + number on the bus).
Since we know from the v1 comments that qemu has a max of 16384 units, add:
Not all hypervisors support larger <code>unit</code> values. It is up to
each hypervisor to determine the maximum value supported for the adapter.
I could add "<code>target</code> and " if you think that would be
better..
The text with both target and unit sounds good to me.
> <p>
> <span class="since">Since 1.2.8</span>, the
<code>source</code>
> element of a SCSI device may contain the
<code>protocol</code>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f0f7400..b3c5cb8 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3833,10 +3833,10 @@
> <ref name="driveBus"/>
> </attribute>
> <attribute name="target">
> - <ref name="driveTarget"/>
> + <ref name="driveSCSITarget"/>
> </attribute>
> <attribute name="unit">
> - <ref name="driveUnit"/>
> + <ref name="driveSCSIUnit"/>
> </attribute>
> </define>
> <define name="usbportaddress">
> @@ -5129,11 +5129,21 @@
> <param name="pattern">[0-9]{1,2}</param>
> </data>
> </define>
> + <define name="driveSCSITarget">
> + <data type="string">
> + <param name="pattern">[0-9]{1,10}</param>
> + </data>
> + </define>
> <define name="driveUnit">
> <data type="string">
> <param name="pattern">[0-9]{1,2}</param>
> </data>
> </define>
> + <define name="driveSCSIUnit">
> + <data type="string">
> + <param name="pattern">[0-9]{1,20}</param>
> + </data>
> + </define>
> <define name="featureName">
> <data type="string">
> <param name='pattern'>[a-zA-Z0-9\-_\.]+</param>
>
I'm going to add a test as well. It's essentially a copy of
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-scsi.xml
adjusted to have a larger unit value (it'll be attached)
Ok. If you push it as a separate patch, feel free to add:
Reviewed-by: Eric Farman <farman(a)linux.vnet.ibm.com>
Thanks,
Eric
10:29 p.m.
New subject: [libvirt] [PATCH 5/5] docs: Correct typos in scsi hostdev and address elements
The type='scsi' parameter of an address element is ignored
if placed within a hostdev section, and rejected by the XML
schema used by virt-xml-validate. Remove it from the doc,
and correct a typo in the remaining address arguments.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
docs/formatdomain.html.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c88c4a6..f97a049 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2839,7 +2839,7 @@
<dd>Drive addresses have the following additional
attributes: <code>controller</code> (a 2-digit controller
number), <code>bus</code> (a 2-digit bus number),
- <code>target</code> (a 2-digit bus number),
+ <code>target</code> (a 2-digit target number),
and <code>unit</code> (a 2-digit unit number on the bus).
</dd>
<dt><code>type='virtio-serial'</code></dt>
@@ -3148,7 +3148,7 @@
<hostdev mode='subsystem' type='scsi' sgio='filtered'
rawio='yes'>
<source>
<adapter name='scsi_host0'/>
- <address type='scsi' bus='0' target='0'
unit='0'/>
+ <address bus='0' target='0' unit='0'/>
</source>
<readonly/>
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
--
1.9.1
1:39 p.m.
New subject: [libvirt] [PATCH 5/5] docs: Correct typos in scsi hostdev and address elements
On 06/16/2015 11:29 PM, Eric Farman wrote:
The type='scsi' parameter of an address element is ignored
if placed within a hostdev section, and rejected by the XML
schema used by virt-xml-validate. Remove it from the doc,
and correct a typo in the remaining address arguments.
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
docs/formatdomain.html.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
ACK
John
9:29 a.m.
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(a)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(-)
9:40 a.m.
On 06/17/2015 04:29 PM, 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.
The recommendation came
from John and since he can push... following it
to get it upstream is ... OK. :-)
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(a)linux.vnet.ibm.com>
I agree with Mat!
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Another side info: I will currently not revert your former patch and
apply this patch set on branches devel and rebased but wait until I
update to libvirt v1.3.0!
If you disagree with this approach please let me know.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
2:17 a.m.
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(a)linux.vnet.ibm.com>
I'm with you.
Reviewed-by: Stefan Zimmermann <stzi(a)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(-)
>
1:49 p.m.
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
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(-)
While I understand the comments regarding the step by step process -
consider bisection when/if something went wrong with these patches in
the future... Was the issue because we parsed a positive integer? Was
the issue because we printed an unsigned value? Was the issue because
we printed a long long unsigned? Easier to revert something smaller and
rework, than have to go through everything.
I've made adjustments in my local branch to add the unsigned for bus and
target, as well as a couple of other adjustments I've noted. I'll give
it a day or so to percolate before pushing though just in case I've
missed something...
Tks -
John
John
3:20 p.m.
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
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(-)
FYI: Just pushed upstream.
John
3433
days inactive
3438
days old
17 comments
5 participants
participants (5)
-
Boris Fiuczynski -
Eric Farman -
John Ferlan -
Matthew Rosato -
Stefan Zimmermann