[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv1 4/9] conf: Add helper for listing domains on drivers supporting virDomainObj



On 06/06/12 00:29, Eric Blake wrote:
diff --git a/src/Makefile.am b/src/Makefile.am
index 5693fb4..fd9d892 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -194,6 +194,9 @@ CPU_CONF_SOURCES =						\
  CONSOLE_CONF_SOURCES =						\
  		conf/virconsole.c conf/virconsole.h

+# Domain listing helpers
+DOMAIN_LIST_SOURCES =						\
+		conf/virdomainlist.c conf/virdomainlist.h
  CONF_SOURCES =							\

While what you have works, I like to add a blank line between any macro
definition that uses \-newline to take up more than one source line, so
that it is a bit more obvious that the last line should not have \ (or
if someone does accidentally put \ on the last line, at least the next
macro name is still an independent macro name rather than an unintended
continuation of the first macro).  That said...

  		$(NETDEV_CONF_SOURCES)				\
  		$(DOMAIN_CONF_SOURCES)				\
@@ -206,7 +209,8 @@ CONF_SOURCES =							\
  		$(INTERFACE_CONF_SOURCES)			\
  		$(SECRET_CONF_SOURCES)				\
  		$(CPU_CONF_SOURCES)				\
-		$(CONSOLE_CONF_SOURCES)
+		$(CONSOLE_CONF_SOURCES)				\
+		$(DOMAIN_LIST_SOURCES)

...I would just inline the listing of the two new files to
DOMAIN_CONF_SOURCES rather than creating a new category DOMAIN_LIST_SOURCES.

The problem with DOMAIN_CONF_SOURCES is that files specified there get built into the libvirt_lxc binary that doesn't include/link libvirt.[ch] which results into a compile failure as virGetDomain is undefined in that case. That's also the reason I used CONSOLE_CONF_SOURCES.



  # The remote RPC driver, covering domains, storage, networks, etc
  REMOTE_DRIVER_GENERATED = \
diff --git a/src/conf/virdomainlist.c b/src/conf/virdomainlist.c
new file mode 100644
index 0000000..180b37d
--- /dev/null
+++ b/src/conf/virdomainlist.c
@@ -0,0 +1,214 @@
+/**
+ * virdomainlist.c: Helpers for listing and filtering domains.
+ *
+ * Copyright (C) 2011-2012 Red Hat, Inc.

Are we really borrowing any contents written in 2011, or can this be
shortened to just 2012?

I just borrowed the license statement and forgot to fix the year :)

Peter


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]