Re: [libvirt] [PATCH] storage pool discovery

2008-08-27 Thread Daniel P. Berrange
On Wed, Aug 27, 2008 at 03:34:14PM -0400, David Lively wrote: > On Wed, 2008-08-27 at 17:51 +0100, Daniel P. Berrange wrote: > > On Mon, Aug 25, 2008 at 10:33:45AM -0400, David Lively wrote: > > > After making the change I suggested above, it now feels a little strange > > > because "Pool" is gone

Re: [libvirt] [PATCH] storage pool discovery

2008-08-27 Thread David Lively
On Wed, 2008-08-27 at 17:51 +0100, Daniel P. Berrange wrote: > On Mon, Aug 25, 2008 at 10:33:45AM -0400, David Lively wrote: > > After making the change I suggested above, it now feels a little strange > > because "Pool" is gone from the name. I'm starting to think > > "*Discover[Storage]PoolSourc

Re: [libvirt] [PATCH] storage pool discovery

2008-08-27 Thread Daniel P. Berrange
On Mon, Aug 25, 2008 at 10:33:45AM -0400, David Lively wrote: > On Fri, 2008-08-22 at 16:15 +0100, Daniel P. Berrange wrote: > > On Fri, Aug 22, 2008 at 10:58:54AM -0400, David Lively wrote: > > > As long as we're on the subject of naming (and before it's too late), > > > it's been bothering me tha

Re: [libvirt] [PATCH] storage pool discovery

2008-08-25 Thread David Lively
On Fri, 2008-08-22 at 16:15 +0100, Daniel P. Berrange wrote: > On Fri, Aug 22, 2008 at 10:58:54AM -0400, David Lively wrote: > > As long as we're on the subject of naming (and before it's too late), > > it's been bothering me that we keep calling this "storage pool > > discovery". To me, "storage

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread Daniel P. Berrange
On Fri, Aug 22, 2008 at 11:06:38PM +0200, Jim Meyering wrote: > David Lively <[EMAIL PROTECTED]> wrote: > ... > >> > AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) > >> > +if test "$with_storage_fs" = "yes"; then > >> > + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sb

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread Jim Meyering
David Lively <[EMAIL PROTECTED]> wrote: ... >> > AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) >> > +if test "$with_storage_fs" = "yes"; then >> > + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin]) >> > + AC_DEFINE_UNQUOTED([SHOWMOUNT], ["$SHOWMOUNT"], >

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread Jim Meyering
David Lively <[EMAIL PROTECTED]> wrote: > On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote: >> > +const char *name, *path; >> >> path can be const, too. > > It is, at least according to my gcc (4.3.0-8, x86_64, Fedora9). > > Compile the following and without FORCE_WARNING to check: > > voi

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread David Lively
On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote: > > +asprintf(&srcSpec, > > + "", > > + hostlen, host, port) : > > Do the hostname and port string need to be quoted (and/or evoke > warning/failure), in case they contain things like "'"? T

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread David Lively
On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote: > > +const char *name, *path; > > path can be const, too. It is, at least according to my gcc (4.3.0-8, x86_64, Fedora9). Compile the following and without FORCE_WARNING to check: void check() { #ifdef FORCE_WARNING const char *foo;

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread David Lively
Hi Jim - Thanks for your comments. I really appreciate the detailed look. I'll implement your suggestions (including the refactoring of the code to turn a virStringList into a single XML string), though I have a question about one of them: On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote:

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread Jim Meyering
David Lively <[EMAIL PROTECTED]> wrote: > Here's the patch with those issues addressed (also merged with latest > upstream - avoids internal.h merge conflict) ... Hi David, Looks good. A couple of minor suggestions and questions. ... > diff --git a/configure.in b/configure.in > index 9479f1c..430

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread Daniel P. Berrange
On Fri, Aug 22, 2008 at 10:58:54AM -0400, David Lively wrote: > On Fri, 2008-08-22 at 09:50 +0100, Daniel P. Berrange wrote: > > > +const char *start_tag = "\n"; > > > +const char *end_tag = "\n"; > > > > I'd prefer that to be- we avoid capitals in the > > XML element names everywhere

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread David Lively
Here's the patch with those issues addressed (also merged with latest upstream - avoids internal.h merge conflict) ... Dave On Fri, 2008-08-22 at 09:50 +0100, Daniel P. Berrange wrote: > On Thu, Aug 21, 2008 at 10:29:43AM -0400, David Lively wrote: > > Hi Folks - > > Here's my second pass at st

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread David Lively
On Fri, 2008-08-22 at 09:50 +0100, Daniel P. Berrange wrote: > > +const char *start_tag = "\n"; > > +const char *end_tag = "\n"; > > I'd prefer that to be- we avoid capitals in the > XML element names everywhere else, and in the few cases of > joining words use an underscore, but I thi

Re: [libvirt] [PATCH] storage pool discovery

2008-08-22 Thread Daniel P. Berrange
On Thu, Aug 21, 2008 at 10:29:43AM -0400, David Lively wrote: > Hi Folks - > Here's my second pass at storage pool discovery. I've taken Daniel's > suggestion and made it return a single XML doc containing > elements rather than an array of docs (and also incorporated > suggestions from Daniel

Re: [libvirt] [PATCH] storage pool discovery

2008-08-06 Thread David Lively
Thanks much for your comments, Jim. They all look reasonable (though I may be intentionally trimming a NL in one of these cases -- I'm checking). And I'll start following the HACKING file recommendations before submitting my next attempt :-) (Though note I'm not yet submitting tests since we hav

Re: [libvirt] [PATCH] storage pool discovery

2008-08-06 Thread David Lively
On Fri, 2008-08-01 at 10:40 +0100, Daniel P. Berrange wrote: > On Wed, Jul 30, 2008 at 04:30:01PM -0400, David Lively wrote: > > Finally, there's an underspecification issue. The XML pool > > descriptions returned are supposed to be usable as valid input to > > virStoragePoolDefineXML. But the

Re: [libvirt] [PATCH] storage pool discovery

2008-08-04 Thread Stefan de Konink
Daniel P. Berrange schreef: On Mon, Aug 04, 2008 at 11:19:25AM +0200, Stefan de Konink wrote: Daniel P. Berrange schreef: On Mon, Aug 04, 2008 at 11:02:33AM +0200, Stefan de Konink wrote: On Mon, 4 Aug 2008, Daniel P. Berrange wrote: - - For iSCSI and related stuff everything was relatively

Re: [libvirt] [PATCH] storage pool discovery

2008-08-04 Thread Daniel P. Berrange
On Mon, Aug 04, 2008 at 11:19:25AM +0200, Stefan de Konink wrote: > Daniel P. Berrange schreef: > >On Mon, Aug 04, 2008 at 11:02:33AM +0200, Stefan de Konink wrote: > >>On Mon, 4 Aug 2008, Daniel P. Berrange wrote: > >> > - - For iSCSI and related stuff everything was relatively easy, because >

Re: [libvirt] [PATCH] storage pool discovery

2008-08-04 Thread Stefan de Konink
Daniel P. Berrange schreef: On Mon, Aug 04, 2008 at 11:02:33AM +0200, Stefan de Konink wrote: On Mon, 4 Aug 2008, Daniel P. Berrange wrote: - - For iSCSI and related stuff everything was relatively easy, because this would just mean to write the right /dev/blabla to the xenstore. What is y

Re: [libvirt] [PATCH] storage pool discovery

2008-08-04 Thread Daniel P. Berrange
On Mon, Aug 04, 2008 at 11:02:33AM +0200, Stefan de Konink wrote: > On Mon, 4 Aug 2008, Daniel P. Berrange wrote: > > > > - - For iSCSI and related stuff everything was relatively easy, because > > > this would just mean to write the right /dev/blabla to the xenstore. > > > What is your idea t

Re: [libvirt] [PATCH] storage pool discovery

2008-08-04 Thread Stefan de Konink
On Mon, 4 Aug 2008, Daniel P. Berrange wrote: > > - - For iSCSI and related stuff everything was relatively easy, because > > this would just mean to write the right /dev/blabla to the xenstore. > > What is your idea to get different drivers working via: > > virt://pool/volume (so basically

Re: [libvirt] [PATCH] storage pool discovery

2008-08-04 Thread Daniel P. Berrange
On Sun, Aug 03, 2008 at 04:20:50AM +0200, Stefan de Konink wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA512 > > Daniel, > > Stefan de Konink schreef: > > As you know I wrote such thing for iSCSI/Solaris, iSCSI/Netapp so... if > > you want such script I can fabricate something, maybe it

Re: [libvirt] [PATCH] storage pool discovery

2008-08-03 Thread Jim Meyering
Hi David, I spotted a few things that would be good to change: David Lively <[EMAIL PROTECTED]> wrote: > diff --git a/configure.in b/configure.in > index 8e04f14..5ef0384 100644 > --- a/configure.in > +++ b/configure.in > @@ -660,6 +660,10 @@ if test "$with_storage_fs" = "yes" -o "$with_storage_f

Re: [libvirt] [PATCH] storage pool discovery

2008-08-02 Thread Stefan de Konink
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Daniel, Stefan de Konink schreef: > As you know I wrote such thing for iSCSI/Solaris, iSCSI/Netapp so... if > you want such script I can fabricate something, maybe it would be > interesting to not create a script, but instead a binary that links to

Re: [libvirt] [PATCH] storage pool discovery

2008-08-01 Thread Stefan de Konink
On Fri, 1 Aug 2008, Daniel P. Berrange wrote: > On Fri, Aug 01, 2008 at 11:46:14AM +0200, Stefan de Konink wrote: > > Daniel, > > > > When this gets in; would it get more easy to lookup a file->storagepool. > > In order to make the alternative configuration I proposed for > > domains working? (Bas

Re: [libvirt] [PATCH] storage pool discovery

2008-08-01 Thread Daniel P. Berrange
On Fri, Aug 01, 2008 at 11:46:14AM +0200, Stefan de Konink wrote: > Daniel, > > When this gets in; would it get more easy to lookup a file->storagepool. > In order to make the alternative configuration I proposed for > domains working? (Basically providing pool/volume instead of filename) I don't

Re: [libvirt] [PATCH] storage pool discovery

2008-08-01 Thread Stefan de Konink
Daniel, When this gets in; would it get more easy to lookup a file->storagepool. In order to make the alternative configuration I proposed for domains working? (Basically providing pool/volume instead of filename) Stefan -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com

Re: [libvirt] [PATCH] storage pool discovery

2008-08-01 Thread Daniel P. Berrange
On Wed, Jul 30, 2008 at 04:30:01PM -0400, David Lively wrote: > For example, to discover existing LVM volume groups, there's no need > to specify a source: > virsh # pool-discover logical > > While discovering nfs shares requires a document to specify > the host to query: > virsh # pool-disco