Re: [libvirt] [PATCH V5 01/12] src/xenxs: Export code for reuse

2014-08-12 Thread Eric Blake 
On 08/11/2014 11:20 PM, Jim Fehlig wrote:
 Kiarie Kahurani wrote:
 wrap code tagged for resuse into one function and export it

 Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 73 
 +++---
  src/xenxs/xen_xm.h |  2 ++
   
 
 This patch got me thinking about the 'xenxs' directory and its
 contents.  IMO, the directory should be named 'xenconfig' since it
 contains parsing/formating functions for the various xen config
 formats.  Its contents should be xen_sxpr.[ch] for sxpr format,
 xen_xm.[ch] for xm format, and xen_xl.[ch] for xl format. 
 xen_common.[ch] would contain parsing/formating functions common to xm
 and xl.  Any opinions on this layout?  I can work on a patch if others
 think this is reasonable.

Sounds fine to me.  And 'git mv' makes it rather easy to do; just make
sure you have 'git config diff.renames true' before sending the patch,
so that the patch is compressed to just the tweaks accounting for the
new names rather than wholesale delete/add actions.

-- 
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 V5 01/12] src/xenxs: Export code for reuse

2014-08-11 Thread Kiarie Kahurani 
wrap code tagged for resuse into one function and export it

Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
---
 src/xenxs/xen_xm.c | 73 +++---
 src/xenxs/xen_xm.h |  2 ++
 2 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index f70b395..8238026 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1263,59 +1263,70 @@ xenParseXMOS(virConfPtr conf, virDomainDefPtr def)
 }
 
 
-/*
- * Turn a config record into a lump of XML describing the
- * domain, suitable for later feeding for virDomainCreateXML
- */
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
-   virCapsPtr caps)
+int
+xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def,
+ virCapsPtr caps, int xendConfigVersion)
 {
-virDomainDefPtr def = NULL;
-
-if (VIR_ALLOC(def)  0)
-return NULL;
-
-def-virtType = VIR_DOMAIN_VIRT_XEN;
-def-id = -1;
 
 if (xenParseXMGeneralMeta(conf, def, caps)  0)
-goto cleanup;
+return -1;
 
 if (xenParseXMOS(conf, def)  0)
-goto cleanup;
+return -1;
 
 if (xenParseXMMem(conf, def)  0)
-goto cleanup;
+return -1;
+
+if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
+return -1;
 
 if (xenParseXMEventsActions(conf, def)  0)
-goto cleanup;
+return -1;
+
+if (xenParseXMPCI(conf, def)  0)
+return -1;
 
 if (xenParseXMCPUFeatures(conf, def)  0)
-goto cleanup;
+return -1;
 
-if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
-goto cleanup;
+if (xenParseXMEmulatedDevices(conf, def)  0)
+return -1;
 
-if (xenParseXMDisk(conf, def, xendConfigVersion)  0)
-goto cleanup;
+if (xenParseXMCharDev(conf, def)  0)
+return -1;
+
+if (xenParseXMVfb(conf, def, xendConfigVersion)  0)
+return -1;
 
 if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
-goto cleanup;
+return -1;
 
 if (xenParseXMVif(conf, def)  0)
-goto cleanup;
+return -1;
 
-if (xenParseXMPCI(conf, def)  0)
-goto cleanup;
+return 0;
+}
 
-if (xenParseXMEmulatedDevices(conf, def)  0)
-goto cleanup;
+/*
+ * Turn a config record into a lump of XML describing the
+ * domain, suitable for later feeding for virDomainCreateXML
+ */
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+   virCapsPtr caps)
+{
+virDomainDefPtr def = NULL;
 
-if (xenParseXMVfb(conf, def, xendConfigVersion)  0)
+if (VIR_ALLOC(def)  0)
+return NULL;
+
+def-virtType = VIR_DOMAIN_VIRT_XEN;
+def-id = -1;
+
+if (xenParseConfigCommon(conf, def, caps, xendConfigVersion)  0)
 goto cleanup;
 
-if (xenParseXMCharDev(conf, def)  0)
+if (xenParseXMDisk(conf, def, xendConfigVersion)  0)
 goto cleanup;
 
 return def;
diff --git a/src/xenxs/xen_xm.h b/src/xenxs/xen_xm.h
index 629a4b3..261ba1f 100644
--- a/src/xenxs/xen_xm.h
+++ b/src/xenxs/xen_xm.h
@@ -35,5 +35,7 @@ virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr 
def,
 
 virDomainDefPtr xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps);
+int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def,
+ virCapsPtr caps, int xendConfigVersion);
 
 #endif /* __VIR_XEN_XM_H__ */
-- 
1.8.4.5

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

Re: [libvirt] [PATCH V5 01/12] src/xenxs: Export code for reuse

2014-08-11 Thread Jim Fehlig 
Kiarie Kahurani wrote:
 wrap code tagged for resuse into one function and export it

 Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 73 
 +++---
  src/xenxs/xen_xm.h |  2 ++
   

This patch got me thinking about the 'xenxs' directory and its
contents.  IMO, the directory should be named 'xenconfig' since it
contains parsing/formating functions for the various xen config
formats.  Its contents should be xen_sxpr.[ch] for sxpr format,
xen_xm.[ch] for xm format, and xen_xl.[ch] for xl format. 
xen_common.[ch] would contain parsing/formating functions common to xm
and xl.  Any opinions on this layout?  I can work on a patch if others
think this is reasonable.

Your patch looks good, but I'll defer pushing it until hearing opinions
on the above suggestion.

Regards,
Jim

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

Re: [libvirt] [PATCH V5 01/12] src/xenxs: Export code for reuse

2014-08-11 Thread David kiarie 
On Tue, Aug 12, 2014 at 12:21 AM, Kiarie Kahurani
davidkiar...@gmail.com wrote:
 wrap code tagged for resuse into one function and export it

 Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 73 
 +++---
  src/xenxs/xen_xm.h |  2 ++
  2 files changed, 44 insertions(+), 31 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index f70b395..8238026 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -1263,59 +1263,70 @@ xenParseXMOS(virConfPtr conf, virDomainDefPtr def)
  }


 -/*
 - * Turn a config record into a lump of XML describing the
 - * domain, suitable for later feeding for virDomainCreateXML
 - */
 -virDomainDefPtr
 -xenParseXM(virConfPtr conf, int xendConfigVersion,
 -   virCapsPtr caps)
 +int
 +xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def,
 + virCapsPtr caps, int xendConfigVersion)
  {
 -virDomainDefPtr def = NULL;
 -
 -if (VIR_ALLOC(def)  0)
 -return NULL;
 -
 -def-virtType = VIR_DOMAIN_VIRT_XEN;
 -def-id = -1;

  if (xenParseXMGeneralMeta(conf, def, caps)  0)
 -goto cleanup;
 +return -1;

  if (xenParseXMOS(conf, def)  0)
 -goto cleanup;
 +return -1;

  if (xenParseXMMem(conf, def)  0)
 -goto cleanup;
 +return -1;
 +
 +if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
 +return -1;

  if (xenParseXMEventsActions(conf, def)  0)
 -goto cleanup;
 +return -1;
 +
 +if (xenParseXMPCI(conf, def)  0)
 +return -1;

  if (xenParseXMCPUFeatures(conf, def)  0)
 -goto cleanup;
 +return -1;

 -if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
 -goto cleanup;
 +if (xenParseXMEmulatedDevices(conf, def)  0)
 +return -1;

 -if (xenParseXMDisk(conf, def, xendConfigVersion)  0)
 -goto cleanup;
 +if (xenParseXMCharDev(conf, def)  0)
 +return -1;
 +
 +if (xenParseXMVfb(conf, def, xendConfigVersion)  0)
 +return -1;

  if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
 -goto cleanup;
 +return -1;

I guess this should come of too; its already taken care of by xenParseXMOS.


  if (xenParseXMVif(conf, def)  0)
 -goto cleanup;
 +return -1;

 -if (xenParseXMPCI(conf, def)  0)
 -goto cleanup;
 +return 0;
 +}

 -if (xenParseXMEmulatedDevices(conf, def)  0)
 -goto cleanup;
 +/*
 + * Turn a config record into a lump of XML describing the
 + * domain, suitable for later feeding for virDomainCreateXML
 + */
 +virDomainDefPtr
 +xenParseXM(virConfPtr conf, int xendConfigVersion,
 +   virCapsPtr caps)
 +{
 +virDomainDefPtr def = NULL;

 -if (xenParseXMVfb(conf, def, xendConfigVersion)  0)
 +if (VIR_ALLOC(def)  0)
 +return NULL;
 +
 +def-virtType = VIR_DOMAIN_VIRT_XEN;
 +def-id = -1;
 +
 +if (xenParseConfigCommon(conf, def, caps, xendConfigVersion)  0)
  goto cleanup;

 -if (xenParseXMCharDev(conf, def)  0)
 +if (xenParseXMDisk(conf, def, xendConfigVersion)  0)
  goto cleanup;

  return def;
 diff --git a/src/xenxs/xen_xm.h b/src/xenxs/xen_xm.h
 index 629a4b3..261ba1f 100644
 --- a/src/xenxs/xen_xm.h
 +++ b/src/xenxs/xen_xm.h
 @@ -35,5 +35,7 @@ virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr 
 def,

  virDomainDefPtr xenParseXM(virConfPtr conf, int xendConfigVersion,
 virCapsPtr caps);
 +int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def,
 + virCapsPtr caps, int xendConfigVersion);

  #endif /* __VIR_XEN_XM_H__ */
 --
 1.8.4.5


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