Re: [libvirt] [PATCH 1/8] hostdev: Introduce virDomainHostdevSubsysUSB
On 07/11/2014 06:35 AM, John Ferlan wrote:
> Create a separate typedef for the hostdev union data describing USB.
> Then adjust the code to use the new pointer
>
> Signed-off-by: John Ferlan
> ---
> src/conf/domain_audit.c | 4 ++--
> src/conf/domain_conf.c | 50
> +++-
> src/conf/domain_conf.h | 22 ++
> src/lxc/lxc_cgroup.c | 4 ++--
> src/lxc/lxc_controller.c | 10
> src/lxc/lxc_driver.c | 16 ++---
> src/qemu/qemu_cgroup.c | 4 ++--
> src/qemu/qemu_command.c | 26 ++---
> src/qemu/qemu_hotplug.c | 7 +++---
> src/security/security_apparmor.c | 6 ++---
> src/security/security_dac.c | 12 --
> src/security/security_selinux.c | 10
> src/security/virt-aa-helper.c| 5 ++--
> src/util/virhostdev.c| 50
> +++-
> 14 files changed, 104 insertions(+), 122 deletions(-)
Mostly mechanical fallout from the type shuffle.
>
> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
> index a3d6c67..8277b06 100644
> --- a/src/conf/domain_audit.c
> +++ b/src/conf/domain_audit.c
> @@ -388,6 +388,7 @@ virDomainAuditHostdev(virDomainObjPtr vm,
> virDomainHostdevDefPtr hostdev,
> char *address = NULL;
> char *device = NULL;
> const char *virt;
> +virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
>
> virUUIDFormat(vm->def->uuid, uuidstr);
> if (!(vmname = virAuditEncode("vm", vm->def->name))) {
> @@ -415,8 +416,7 @@ virDomainAuditHostdev(virDomainObjPtr vm,
> virDomainHostdevDefPtr hostdev,
> break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> if (virAsprintfQuiet(&address, "%.3d.%.3d",
> - hostdev->source.subsys.u.usb.bus,
> - hostdev->source.subsys.u.usb.device) < 0) {
> + usbsrc->bus, usbsrc->device) < 0) {
I also find it more legible :)
> @@ -10104,15 +10102,18 @@ static int
> virDomainHostdevMatchSubsysUSB(virDomainHostdevDefPtr a,
> virDomainHostdevDefPtr b)
> {
> -if (a->source.subsys.u.usb.bus && a->source.subsys.u.usb.device) {
> +virDomainHostdevSubsysUSBPtr ausbsrc = &a->source.subsys.u.usb;
> +virDomainHostdevSubsysUSBPtr busbsrc = &b->source.subsys.u.usb;
I read the second variable as bus_b_src, and tried to figure out what
the first one was (was "aus" a typo for "bus"?). Until I saw that you
meant b_usb_src. Might be worth an _ in the naming to keep it easier to
read. Or even different naming conventions: first, second.
> +++ b/src/conf/domain_conf.h
> @@ -390,20 +390,24 @@ typedef enum {
>
> VIR_ENUM_DECL(virDomainHostdevSubsysPCIBackend)
>
> +typedef struct _virDomainHostdevSubsysUSB virDomainHostdevSubsysUSB;
> +typedef virDomainHostdevSubsysUSB *virDomainHostdevSubsysUSBPtr;
> +struct _virDomainHostdevSubsysUSB {
> +bool autoAddress; /* bus/device were filled automatically based
> + on vedor/product */
Pre-existing typo, but you might as well fix it during the code motion:
> +unsigned bus;
> +unsigned device;
> +
> +unsigned vendor;
> +unsigned product;
> +};
> +
> typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys;
> typedef virDomainHostdevSubsys *virDomainHostdevSubsysPtr;
> struct _virDomainHostdevSubsys {
> int type; /* enum virDomainHostdevSubsysType */
> union {
> -struct {
> -bool autoAddress; /* bus/device were filled automatically based
> - on vedor/product */
s/vedor/vendor/
ACK with nits fixed.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/8] hostdev: Introduce virDomainHostdevSubsysUSB
Create a separate typedef for the hostdev union data describing USB.
Then adjust the code to use the new pointer
Signed-off-by: John Ferlan
---
src/conf/domain_audit.c | 4 ++--
src/conf/domain_conf.c | 50 +++-
src/conf/domain_conf.h | 22 ++
src/lxc/lxc_cgroup.c | 4 ++--
src/lxc/lxc_controller.c | 10
src/lxc/lxc_driver.c | 16 ++---
src/qemu/qemu_cgroup.c | 4 ++--
src/qemu/qemu_command.c | 26 ++---
src/qemu/qemu_hotplug.c | 7 +++---
src/security/security_apparmor.c | 6 ++---
src/security/security_dac.c | 12 --
src/security/security_selinux.c | 10
src/security/virt-aa-helper.c| 5 ++--
src/util/virhostdev.c| 50 +++-
14 files changed, 104 insertions(+), 122 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index a3d6c67..8277b06 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -388,6 +388,7 @@ virDomainAuditHostdev(virDomainObjPtr vm,
virDomainHostdevDefPtr hostdev,
char *address = NULL;
char *device = NULL;
const char *virt;
+virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
virUUIDFormat(vm->def->uuid, uuidstr);
if (!(vmname = virAuditEncode("vm", vm->def->name))) {
@@ -415,8 +416,7 @@ virDomainAuditHostdev(virDomainObjPtr vm,
virDomainHostdevDefPtr hostdev,
break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
if (virAsprintfQuiet(&address, "%.3d.%.3d",
- hostdev->source.subsys.u.usb.bus,
- hostdev->source.subsys.u.usb.device) < 0) {
+ usbsrc->bus, usbsrc->device) < 0) {
VIR_WARN("OOM while encoding audit message");
goto cleanup;
}
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b91ccf7..046b4f8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3800,6 +3800,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
xmlNodePtr cur;
char *startupPolicy = NULL;
char *autoAddress;
+virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
def->startupPolicy =
@@ -3816,7 +3817,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
if ((autoAddress = virXMLPropString(node, "autoAddress"))) {
if (STREQ(autoAddress, "yes"))
-def->source.subsys.u.usb.autoAddress = true;
+usbsrc->autoAddress = true;
VIR_FREE(autoAddress);
}
@@ -3833,8 +3834,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
if (vendor) {
got_vendor = true;
-if (virStrToLong_ui(vendor, NULL, 0,
-&def->source.subsys.u.usb.vendor) < 0) {
+if (virStrToLong_ui(vendor, NULL, 0, &usbsrc->vendor) < 0)
{
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse vendor id %s"), vendor);
VIR_FREE(vendor);
@@ -3852,7 +3852,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
if (product) {
got_product = true;
if (virStrToLong_ui(product, NULL, 0,
-&def->source.subsys.u.usb.product) <
0) {
+&usbsrc->product) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse product %s"),
product);
@@ -3870,8 +3870,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
bus = virXMLPropString(cur, "bus");
if (bus) {
-if (virStrToLong_ui(bus, NULL, 0,
-&def->source.subsys.u.usb.bus) < 0) {
+if (virStrToLong_ui(bus, NULL, 0, &usbsrc->bus) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse bus %s"), bus);
VIR_FREE(bus);
@@ -3886,8 +3885,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
device = virXMLPropString(cur, "device");
if (device) {
-if (virStrToLong_ui(device, NULL, 0,
-&def->source.subsys.u.usb.device) < 0)
{
+if (virStrToLong_ui(device, NULL, 0, &usbsrc->device) < 0)
{
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse device %s"),
