Re: [libvirt] [PATCH 1/8] hostdev: Introduce virDomainHostdevSubsysUSB

2014-07-21 Thread Eric Blake
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 jfer...@redhat.com
 ---
  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

2014-07-11 Thread John Ferlan
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 jfer...@redhat.com
---
 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),
device);
@@ -3910,7 +3908,7 @@