Re: [libvirt] [PATCH v2 05/10] utils: util functions for scsi hostdev

2013-04-04 Thread Han Cheng

On 04/03/2013 01:06 PM, Hu Tao wrote:

On Mon, Apr 01, 2013 at 08:00:57PM +0800, Han Cheng wrote:

+int
+virSCSIDeviceListAdd(virSCSIDeviceListPtr list,
+ virSCSIDevicePtr dev)
+{
+if (virSCSIDeviceListFind(list, dev)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Device %s is already in use),
+   dev-name);
+return -1;
+}
+
+if (VIR_REALLOC_N(list-devs, list-count+1)  0) {
+virReportOOMError();
+return -1;
+}
+
+list-devs[list-count++] = dev;


The list is lockable, but you're not protecting it with lock.


Yes.
But for most time, we are manipulating a local list. There is no 
concurrency problem.
The only concurrency problem is about driver-activeScsiHostdevs(type of 
virSCSIDeviceListPtr). We'll hold the lock if we want manipulate that.


Cheng

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


Re: [libvirt] [PATCH v2 05/10] utils: util functions for scsi hostdev

2013-04-04 Thread Han Cheng

On 04/03/2013 05:04 PM, Osier Yang wrote:

On 01/04/13 20:00, Han Cheng wrote:

+ if (virAsprintf(tmp, SCSI_DEVFS /%s/vendor, dev-name)  0) {
+ virReportOOMError();
+ goto out;
+ }
+ if (virFileReadAll(tmp, 1024, vendor)  0)
+ goto out;
+ VIR_FREE(tmp);
+ tmp = NULL;
+ if (virAsprintf(tmp, SCSI_DEVFS /%s/model, dev-name)  0) {
+ virReportOOMError();
+ goto out;
+ }
+ if (virFileReadAll(tmp, 1024, model)  0)
+ goto out;
+ *(vendor + strlen(vendor) - 1) = '\0';


This should be right after reading vendor


+ *(model + strlen(model) - 1) = '\0';
+ if (virAsprintf(dev-id, %s %s, vendor, model)  0) {


Is it possible for the vendor and name contains white space(s)? if
it is,
then separating them by one space has problem.


Actually, it is due to my tests which are done before sending thest 
patches out.
I thought this may not be a big problem. It seems not. So we need to 
virTrimSpaces to trim the spaces.


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


Re: [libvirt] [PATCH v2 05/10] utils: util functions for scsi hostdev

2013-04-03 Thread Han Cheng



On 04/03/2013 01:06 PM, Hu Tao wrote:

On Mon, Apr 01, 2013 at 08:00:57PM +0800, Han Cheng wrote:

+struct _virSCSIDevice {
+unsigned int  adapter;
+unsigned int  bus;
+unsigned int  target;
+unsigned int  unit;
+
+char  *name;   /* adapter:bus:target:unit */
+char  *id; /* model vendor */
+char  *path;
+const char*used_by;   /* name of the domain using this dev */
+
+unsigned int  readonly : 1;
+};
+
+struct _virSCSIDeviceList {
+virObjectLockable parent;
+unsigned int count;
+virSCSIDevicePtr *devs;
+};


I think it's better to implement a generic object list, otherwise
everytime who wants a list, he/she has to re-implement a list.


I agree with you. And I mentioned it in the cover letter.


+
+static virClassPtr virSCSIDeviceListClass;
+
+static void virSCSIDeviceListDispose(void *obj);
+
+static int virSCSIOnceInit(void)
+{
+if (!(virSCSIDeviceListClass = virClassNew(virClassForObjectLockable(),
+  virSCSIDeviceList,
+  sizeof(virSCSIDeviceList),
+  virSCSIDeviceListDispose)))


The indentation style is:

   virClassNew(...
   ...
   ^

Please correct them everywhere in your patches.  You want to have a look
at HACKING for configs of editors.


Sorry about this. This will be fixed.


+
+int virSCSIDeviceFileIterate(virSCSIDevicePtr dev,
+ virSCSIDeviceFileActor actor,
+ void *opaque)
+{
+return (actor)(dev, dev-path, opaque);
+}


What's the difference with directly calling actor?


To be honest, I don't know the difference. Maybe there is no difference.
I just define and use it like vir(pci|usb).c

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


Re: [libvirt] [PATCH v2 05/10] utils: util functions for scsi hostdev

2013-04-03 Thread Osier Yang

On 01/04/13 20:00, Han Cheng wrote:

This patch add util functions for scsi hostdev.

Signed-off-by: Han Cheng hanc.f...@cn.fujitsu.com
---
  po/POTFILES.in   |1 +
  src/Makefile.am  |1 +
  src/libvirt_private.syms |   22 +++
  src/util/virscsi.c   |  399 ++
  src/util/virscsi.h   |   83 ++
  5 files changed, 506 insertions(+), 0 deletions(-)
  create mode 100644 src/util/virscsi.c
  create mode 100644 src/util/virscsi.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 91e5c02..39a0a19 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -174,6 +174,7 @@ src/util/virportallocator.c
  src/util/virprocess.c
  src/util/virrandom.c
  src/util/virsexpr.c
+src/util/virscsi.c
  src/util/virsocketaddr.c
  src/util/virstatslinux.c
  src/util/virstoragefile.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 3f69d39..49d7f88 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -111,6 +111,7 @@ UTIL_SOURCES =  
\
util/virportallocator.c util/virportallocator.h \
util/virprocess.c util/virprocess.h \
util/virrandom.h util/virrandom.c   \
+   util/virscsi.c util/virscsi.h   \
util/virsexpr.c util/virsexpr.h \
util/virsocketaddr.h util/virsocketaddr.c   \
util/virstatslinux.c util/virstatslinux.h   \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f2eefc3..6a5962e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1665,6 +1665,28 @@ virRandomGenerateWWN;
  virRandomInt;
  
  
+# util/virscsi.h

+virSCSIDeviceFileIterate;
+virSCSIDeviceFree;
+virSCSIDeviceGetAdapter;
+virSCSIDeviceGetBus;
+virSCSIDeviceGetDevStr;
+virSCSIDeviceGetName;
+virSCSIDeviceGetReadonly;
+virSCSIDeviceGetTarget;
+virSCSIDeviceGetUnit;
+virSCSIDeviceGetUsedBy;
+virSCSIDeviceListAdd;
+virSCSIDeviceListCount;
+virSCSIDeviceListDel;
+virSCSIDeviceListFind;
+virSCSIDeviceListGet;
+virSCSIDeviceListNew;
+virSCSIDeviceListSteal;
+virSCSIDeviceNew;
+virSCSIDeviceSetUsedBy;
+
+
  # util/virsexpr.h
  sexpr2string;
  sexpr_append;
diff --git a/src/util/virscsi.c b/src/util/virscsi.c
new file mode 100644
index 000..5d0dcd7
--- /dev/null
+++ b/src/util/virscsi.c
@@ -0,0 +1,399 @@
+/*
+ * virscsi.c: helper APIs for managing host SCSI devices
+ *
+ * Copyright (C) 2013 Fujitsu, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Authors:
+ * Han Cheng hanc.f...@cn.fujitsu.com
+ */
+
+#include config.h
+
+#include dirent.h
+#include fcntl.h
+#include inttypes.h
+#include limits.h
+#include stdio.h
+#include string.h
+#include sys/types.h
+#include sys/stat.h
+#include unistd.h
+
+#include virscsi.h
+#include virlog.h
+#include viralloc.h
+#include virutil.h
+#include virerror.h
+
+#define SCSI_DEVFS /sys/bus/scsi/devices


Generally we name macro like this as SYSFS_SCSI_DEVICES.  virpci.c
is too old.


+
+/* For virReportOOMError()  and virReportSystemError() */
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+struct _virSCSIDevice {
+unsigned int  adapter;
+unsigned int  bus;
+unsigned int  target;
+unsigned int  unit;
+
+char  *name;   /* adapter:bus:target:unit */
+char  *id; /* model vendor */
+char  *path;
+const char*used_by;   /* name of the domain using this dev */
+
+unsigned int  readonly : 1;
+};
+
+struct _virSCSIDeviceList {
+virObjectLockable parent;
+unsigned int count;
+virSCSIDevicePtr *devs;
+};
+
+static virClassPtr virSCSIDeviceListClass;
+
+static void virSCSIDeviceListDispose(void *obj);
+
+static int virSCSIOnceInit(void)
+{
+if (!(virSCSIDeviceListClass = virClassNew(virClassForObjectLockable(),
+  virSCSIDeviceList,
+  sizeof(virSCSIDeviceList),
+  virSCSIDeviceListDispose)))
+return -1;
+
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virSCSI)
+
+static int virSCSIDeviceGetAdapterId(char *adapter, unsigned int *adapterid)
+{
+if (STRSKIP(adapter, scsi_host) 
+virStrToLong_ui(adapter + 

Re: [libvirt] [PATCH v2 05/10] utils: util functions for scsi hostdev

2013-04-02 Thread Hu Tao
On Mon, Apr 01, 2013 at 08:00:57PM +0800, Han Cheng wrote:
 This patch add util functions for scsi hostdev.
 
 Signed-off-by: Han Cheng hanc.f...@cn.fujitsu.com
 ---
  po/POTFILES.in   |1 +
  src/Makefile.am  |1 +
  src/libvirt_private.syms |   22 +++
  src/util/virscsi.c   |  399 
 ++
  src/util/virscsi.h   |   83 ++
  5 files changed, 506 insertions(+), 0 deletions(-)
  create mode 100644 src/util/virscsi.c
  create mode 100644 src/util/virscsi.h
 
 diff --git a/po/POTFILES.in b/po/POTFILES.in
 index 91e5c02..39a0a19 100644
 --- a/po/POTFILES.in
 +++ b/po/POTFILES.in
 @@ -174,6 +174,7 @@ src/util/virportallocator.c
  src/util/virprocess.c
  src/util/virrandom.c
  src/util/virsexpr.c
 +src/util/virscsi.c
  src/util/virsocketaddr.c
  src/util/virstatslinux.c
  src/util/virstoragefile.c
 diff --git a/src/Makefile.am b/src/Makefile.am
 index 3f69d39..49d7f88 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -111,6 +111,7 @@ UTIL_SOURCES =
 \
   util/virportallocator.c util/virportallocator.h \
   util/virprocess.c util/virprocess.h \
   util/virrandom.h util/virrandom.c   \
 + util/virscsi.c util/virscsi.h   \
   util/virsexpr.c util/virsexpr.h \
   util/virsocketaddr.h util/virsocketaddr.c   \
   util/virstatslinux.c util/virstatslinux.h   \
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index f2eefc3..6a5962e 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1665,6 +1665,28 @@ virRandomGenerateWWN;
  virRandomInt;
  
  
 +# util/virscsi.h
 +virSCSIDeviceFileIterate;
 +virSCSIDeviceFree;
 +virSCSIDeviceGetAdapter;
 +virSCSIDeviceGetBus;
 +virSCSIDeviceGetDevStr;
 +virSCSIDeviceGetName;
 +virSCSIDeviceGetReadonly;
 +virSCSIDeviceGetTarget;
 +virSCSIDeviceGetUnit;
 +virSCSIDeviceGetUsedBy;
 +virSCSIDeviceListAdd;
 +virSCSIDeviceListCount;
 +virSCSIDeviceListDel;
 +virSCSIDeviceListFind;
 +virSCSIDeviceListGet;
 +virSCSIDeviceListNew;
 +virSCSIDeviceListSteal;
 +virSCSIDeviceNew;
 +virSCSIDeviceSetUsedBy;
 +
 +
  # util/virsexpr.h
  sexpr2string;
  sexpr_append;
 diff --git a/src/util/virscsi.c b/src/util/virscsi.c
 new file mode 100644
 index 000..5d0dcd7
 --- /dev/null
 +++ b/src/util/virscsi.c
 @@ -0,0 +1,399 @@
 +/*
 + * virscsi.c: helper APIs for managing host SCSI devices
 + *
 + * Copyright (C) 2013 Fujitsu, Inc.
 + *
 + * This library is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU Lesser General Public
 + * License as published by the Free Software Foundation; either
 + * version 2.1 of the License, or (at your option) any later version.
 + *
 + * This library is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * Lesser General Public License for more details.
 + *
 + * You should have received a copy of the GNU Lesser General Public
 + * License along with this library.  If not, see
 + * http://www.gnu.org/licenses/.
 + *
 + * Authors:
 + * Han Cheng hanc.f...@cn.fujitsu.com
 + */
 +
 +#include config.h
 +
 +#include dirent.h
 +#include fcntl.h
 +#include inttypes.h
 +#include limits.h
 +#include stdio.h
 +#include string.h
 +#include sys/types.h
 +#include sys/stat.h
 +#include unistd.h
 +
 +#include virscsi.h
 +#include virlog.h
 +#include viralloc.h
 +#include virutil.h
 +#include virerror.h
 +
 +#define SCSI_DEVFS /sys/bus/scsi/devices
 +
 +/* For virReportOOMError()  and virReportSystemError() */
 +#define VIR_FROM_THIS VIR_FROM_NONE
 +
 +struct _virSCSIDevice {
 +unsigned int  adapter;
 +unsigned int  bus;
 +unsigned int  target;
 +unsigned int  unit;
 +
 +char  *name;   /* adapter:bus:target:unit */
 +char  *id; /* model vendor */
 +char  *path;
 +const char*used_by;   /* name of the domain using this dev */
 +
 +unsigned int  readonly : 1;
 +};
 +
 +struct _virSCSIDeviceList {
 +virObjectLockable parent;
 +unsigned int count;
 +virSCSIDevicePtr *devs;
 +};

I think it's better to implement a generic object list, otherwise
everytime who wants a list, he/she has to re-implement a list.

 +
 +static virClassPtr virSCSIDeviceListClass;
 +
 +static void virSCSIDeviceListDispose(void *obj);
 +
 +static int virSCSIOnceInit(void)
 +{
 +if (!(virSCSIDeviceListClass = virClassNew(virClassForObjectLockable(),
 +  virSCSIDeviceList,
 +  sizeof(virSCSIDeviceList),
 +  virSCSIDeviceListDispose)))

The indentation style is:

  virClassNew(...
  

[libvirt] [PATCH v2 05/10] utils: util functions for scsi hostdev

2013-04-01 Thread Han Cheng
This patch add util functions for scsi hostdev.

Signed-off-by: Han Cheng hanc.f...@cn.fujitsu.com
---
 po/POTFILES.in   |1 +
 src/Makefile.am  |1 +
 src/libvirt_private.syms |   22 +++
 src/util/virscsi.c   |  399 ++
 src/util/virscsi.h   |   83 ++
 5 files changed, 506 insertions(+), 0 deletions(-)
 create mode 100644 src/util/virscsi.c
 create mode 100644 src/util/virscsi.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 91e5c02..39a0a19 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -174,6 +174,7 @@ src/util/virportallocator.c
 src/util/virprocess.c
 src/util/virrandom.c
 src/util/virsexpr.c
+src/util/virscsi.c
 src/util/virsocketaddr.c
 src/util/virstatslinux.c
 src/util/virstoragefile.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 3f69d39..49d7f88 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -111,6 +111,7 @@ UTIL_SOURCES =  
\
util/virportallocator.c util/virportallocator.h \
util/virprocess.c util/virprocess.h \
util/virrandom.h util/virrandom.c   \
+   util/virscsi.c util/virscsi.h   \
util/virsexpr.c util/virsexpr.h \
util/virsocketaddr.h util/virsocketaddr.c   \
util/virstatslinux.c util/virstatslinux.h   \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f2eefc3..6a5962e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1665,6 +1665,28 @@ virRandomGenerateWWN;
 virRandomInt;
 
 
+# util/virscsi.h
+virSCSIDeviceFileIterate;
+virSCSIDeviceFree;
+virSCSIDeviceGetAdapter;
+virSCSIDeviceGetBus;
+virSCSIDeviceGetDevStr;
+virSCSIDeviceGetName;
+virSCSIDeviceGetReadonly;
+virSCSIDeviceGetTarget;
+virSCSIDeviceGetUnit;
+virSCSIDeviceGetUsedBy;
+virSCSIDeviceListAdd;
+virSCSIDeviceListCount;
+virSCSIDeviceListDel;
+virSCSIDeviceListFind;
+virSCSIDeviceListGet;
+virSCSIDeviceListNew;
+virSCSIDeviceListSteal;
+virSCSIDeviceNew;
+virSCSIDeviceSetUsedBy;
+
+
 # util/virsexpr.h
 sexpr2string;
 sexpr_append;
diff --git a/src/util/virscsi.c b/src/util/virscsi.c
new file mode 100644
index 000..5d0dcd7
--- /dev/null
+++ b/src/util/virscsi.c
@@ -0,0 +1,399 @@
+/*
+ * virscsi.c: helper APIs for managing host SCSI devices
+ *
+ * Copyright (C) 2013 Fujitsu, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Authors:
+ * Han Cheng hanc.f...@cn.fujitsu.com
+ */
+
+#include config.h
+
+#include dirent.h
+#include fcntl.h
+#include inttypes.h
+#include limits.h
+#include stdio.h
+#include string.h
+#include sys/types.h
+#include sys/stat.h
+#include unistd.h
+
+#include virscsi.h
+#include virlog.h
+#include viralloc.h
+#include virutil.h
+#include virerror.h
+
+#define SCSI_DEVFS /sys/bus/scsi/devices
+
+/* For virReportOOMError()  and virReportSystemError() */
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+struct _virSCSIDevice {
+unsigned int  adapter;
+unsigned int  bus;
+unsigned int  target;
+unsigned int  unit;
+
+char  *name;   /* adapter:bus:target:unit */
+char  *id; /* model vendor */
+char  *path;
+const char*used_by;   /* name of the domain using this dev */
+
+unsigned int  readonly : 1;
+};
+
+struct _virSCSIDeviceList {
+virObjectLockable parent;
+unsigned int count;
+virSCSIDevicePtr *devs;
+};
+
+static virClassPtr virSCSIDeviceListClass;
+
+static void virSCSIDeviceListDispose(void *obj);
+
+static int virSCSIOnceInit(void)
+{
+if (!(virSCSIDeviceListClass = virClassNew(virClassForObjectLockable(),
+  virSCSIDeviceList,
+  sizeof(virSCSIDeviceList),
+  virSCSIDeviceListDispose)))
+return -1;
+
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virSCSI)
+
+static int virSCSIDeviceGetAdapterId(char *adapter, unsigned int *adapterid)
+{
+if (STRSKIP(adapter, scsi_host) 
+virStrToLong_ui(adapter + strlen(scsi_host), NULL, 0,
+adapterid)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Cannot