[PATCH v2 1/8] pool-wizard: Add new line in error message of _create_dir method

[Jiri Prochazka]

New line was missing in print of an error message of _create_dir method

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Wizard.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
index f2c645c..51856e8 100644
--- a/lnst/Controller/Wizard.py
+++ b/lnst/Controller/Wizard.py
@@ -186,7 +186,7 @@ class Wizard:
             print("Dir '%s' has been created" % pool_dir)
             return pool_dir
         except:
-            sys.stderr.write("Failed creating dir")
+            sys.stderr.write("Failed creating dir\n")
             return None
 
     def _create_xml(self, machine_interfaces, hostname,
-- 
2.4.3

[PATCH v2 2/8] pool-wizard: Rework and rename method _query_pool_dir to _check_and_query_pool_dir

[Jiri Prochazka]

This patch introduces several changes to the _query_pool_dir method in
lnst.Controller.Wizard:
- Method was renamed to _check_and_query_pool_dir to better express nature of the method
- Method now uses 1 optional argument - pool_dir - if set to something else than None,
  it checks if it's valid path to existing or creatable dir, if it is, uses it,
  else queries user for a new path (which is also checked)
- Method noninteractive has added default value (None) for argument pool_dir

This method was reworked to prevent code duplication which would occur when mode for
virtual
machines would be introduced

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Wizard.py | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
index 51856e8..b35b216 100644
--- a/lnst/Controller/Wizard.py
+++ b/lnst/Controller/Wizard.py
@@ -32,22 +32,7 @@ class Wizard:
         """ Starts Wizard in an interactive mode
         @param pool_dir Path to pool directory (optional)
         """
-        if pool_dir is None:
-            pool_dir = self._query_pool_dir()
-        else:
-            rv = self._check_path(pool_dir)
-            if rv == PATH_IS_DIR_ACCESSIBLE:
-                print("Pool directory set to '%s'" % pool_dir)
-            elif rv == PATH_DOES_NOT_EXIST:
-                sys.stderr.write("Path '%s' does not exist\n" %
pool_dir)
-                pool_dir = self._query_pool_dir()
-            elif rv == PATH_NOT_DIR:
-                sys.stderr.write("Path '%s' exists but is not a
directory\n"
-                                 % pool_dir)
-                pool_dir = self._query_pool_dir()
-            elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
-                sys.stderr.write("Directory '%s' is not writable\n" %
pool_dir)
-                pool_dir = self._query_pool_dir()
+        pool_dir = self._check_and_query_pool_dir(pool_dir)
 
         while True:
             hostname = self._query_hostname()
@@ -75,7 +60,7 @@ class Wizard:
             else:
                 break
 
-    def noninteractive(self, hostlist, pool_dir):
+    def noninteractive(self, hostlist, pool_dir=None):
         """ Starts Wizard in noninteractive mode
         @param hostlist List of hosts (mandatory)
         @param pool_dir Path to pool_directory (optional)
@@ -330,13 +315,15 @@ class Wizard:
                 sys.stderr.write("Hostname '%s' is not translatable into a
"
                                  "valid IP address\n" % hostname)
 
-    def _query_pool_dir(self):
+    def _check_and_query_pool_dir(self, pool_dir):
         """ Queries user for pool directory
+        @param pool_dir Optional pool_dir which will be checked and used if OK
         @return Valid (is writable by user) path to directory
         """
         while True:
-            pool_dir = raw_input("Enter path to a pool directory "
-                                 "(default: '%s'): " % DefaultPoolDir)
+            if pool_dir is None:
+                pool_dir = raw_input("Enter path to a pool directory "
+                                     "(default: '%s'): " %
DefaultPoolDir)
             if pool_dir == "":
                 pool_dir = DefaultPoolDir
 
@@ -359,6 +346,7 @@ class Wizard:
             elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
                 sys.stderr.write("Directory '%s' is not writable\n"
                                  % pool_dir)
+            pool_dir = None
 
     def _query_port(self):
         """ Queries user for port
-- 
2.4.3

[PATCH v2 2/8] pool-wizard: Rework and rename method _query_pool_dir to _check_and_query_pool_dir

[Ondrej Lichtner]

On Fri, Aug 28, 2015 at 11:17:18AM +0200, Jiri Prochazka wrote:
>  This patch introduces several changes to the _query_pool_dir method
> in lnst.Controller.Wizard:
>  - Method was renamed to _check_and_query_pool_dir to better express nature of the method
>  - Method now uses 1 optional argument - pool_dir - if set to something else than None,
>    it checks if it's valid path to existing or creatable dir, if it is, uses it,
>    else queries user for a new path (which is also checked)
>  - Method noninteractive has added default value (None) for argument pool_dir
>  
>  This method was reworked to prevent code duplication which would occur when mode for
> virtual
>  machines would be introduced
>  
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  ---
>   lnst/Controller/Wizard.py | 28 ++++++++--------------------
>   1 file changed, 8 insertions(+), 20 deletions(-)
>  
>  diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
>  index 51856e8..b35b216 100644
>  --- a/lnst/Controller/Wizard.py
>  +++ b/lnst/Controller/Wizard.py
>  @@ -32,22 +32,7 @@ class Wizard:
>           """ Starts Wizard in an interactive mode
>           @param pool_dir Path to pool directory (optional)
>           """
>  -        if pool_dir is None:
>  -            pool_dir = self._query_pool_dir()
>  -        else:
>  -            rv = self._check_path(pool_dir)
>  -            if rv == PATH_IS_DIR_ACCESSIBLE:
>  -                print("Pool directory set to '%s'" % pool_dir)
>  -            elif rv == PATH_DOES_NOT_EXIST:
>  -                sys.stderr.write("Path '%s' does not exist\n" %
> pool_dir)
>  -                pool_dir = self._query_pool_dir()
>  -            elif rv == PATH_NOT_DIR:
>  -                sys.stderr.write("Path '%s' exists but is not a
> directory\n"
>  -                                 % pool_dir)
>  -                pool_dir = self._query_pool_dir()
>  -            elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
>  -                sys.stderr.write("Directory '%s' is not writable\n" %
> pool_dir)
>  -                pool_dir = self._query_pool_dir()
>  +        pool_dir = self._check_and_query_pool_dir(pool_dir)
>   
>           while True:
>               hostname = self._query_hostname()
>  @@ -75,7 +60,7 @@ class Wizard:
>               else:
>                   break
>   
>  -    def noninteractive(self, hostlist, pool_dir):
>  +    def noninteractive(self, hostlist, pool_dir=None): 
1. Why "None" and not "DefaultPoolDir" ?
2. Doesn't it make more sense to split this into it's own patch? It
doesn't look like it's logically connected to anything else changed in
this patch.

>           """ Starts Wizard in noninteractive mode
>           @param hostlist List of hosts (mandatory)
>           @param pool_dir Path to pool_directory (optional)
>  @@ -330,13 +315,15 @@ class Wizard:
>                   sys.stderr.write("Hostname '%s' is not translatable into a
> "
>                                    "valid IP address\n" % hostname)
>   
>  -    def _query_pool_dir(self):
>  +    def _check_and_query_pool_dir(self, pool_dir):
>           """ Queries user for pool directory
>  +        @param pool_dir Optional pool_dir which will be checked and used if OK
>           @return Valid (is writable by user) path to directory
>           """
>           while True:
>  -            pool_dir = raw_input("Enter path to a pool directory "
>  -                                 "(default: '%s'): " %
> DefaultPoolDir)
>  +            if pool_dir is None:
>  +                pool_dir = raw_input("Enter path to a pool directory "
>  +                                     "(default: '%s'): " %
> DefaultPoolDir)
>               if pool_dir == "":
>                   pool_dir = DefaultPoolDir
>   
>  @@ -359,6 +346,7 @@ class Wizard:
>               elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
>                   sys.stderr.write("Directory '%s' is not writable\n"
>                                    % pool_dir)
>  +            pool_dir = None
>   
>       def _query_port(self):
>           """ Queries user for port
>  -- 
>  2.4.3
>  
>  _______________________________________________
>  LNST-developers mailing list
>  LNST-developers(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/mailman/listinfo/lnst-developers

[PATCH v2 2/8] pool-wizard: Rework and rename method _query_pool_dir to _check_and_query_pool_dir

[Jiri Prochazka]

2015-09-02 15:18 GMT+02:00 Ondrej Lichtner <olichtne(a)redhat.com&gt;:

>  On Fri, Aug 28, 2015 at 11:17:18AM +0200, Jiri Prochazka wrote:
>  > This patch introduces several changes to the _query_pool_dir method in
>  lnst.Controller.Wizard:
>  > - Method was renamed to _check_and_query_pool_dir to better express
>  nature of the method
>  > - Method now uses 1 optional argument - pool_dir - if set to something
>  else than None,
>  >   it checks if it's valid path to existing or creatable dir, if it is,
>  uses it,
>  >   else queries user for a new path (which is also checked)
>  > - Method noninteractive has added default value (None) for argument
>  pool_dir
>  >
>  > This method was reworked to prevent code duplication which would occur
>  when mode for virtual
>  > machines would be introduced
>  >
>  > Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  > ---
>  >  lnst/Controller/Wizard.py | 28 ++++++++--------------------
>  >  1 file changed, 8 insertions(+), 20 deletions(-)
>  >
>  > diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
>  > index 51856e8..b35b216 100644
>  > --- a/lnst/Controller/Wizard.py
>  > +++ b/lnst/Controller/Wizard.py
>  > @@ -32,22 +32,7 @@ class Wizard:
>  >          """ Starts Wizard in an interactive mode
>  >          @param pool_dir Path to pool directory (optional)
>  >          """
>  > -        if pool_dir is None:
>  > -            pool_dir = self._query_pool_dir()
>  > -        else:
>  > -            rv = self._check_path(pool_dir)
>  > -            if rv == PATH_IS_DIR_ACCESSIBLE:
>  > -                print("Pool directory set to '%s'" % pool_dir)
>  > -            elif rv == PATH_DOES_NOT_EXIST:
>  > -                sys.stderr.write("Path '%s' does not exist\n" %
>  pool_dir)
>  > -                pool_dir = self._query_pool_dir()
>  > -            elif rv == PATH_NOT_DIR:
>  > -                sys.stderr.write("Path '%s' exists but is not a
>  directory\n"
>  > -                                 % pool_dir)
>  > -                pool_dir = self._query_pool_dir()
>  > -            elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
>  > -                sys.stderr.write("Directory '%s' is not
> writable\n" %
>  pool_dir)
>  > -                pool_dir = self._query_pool_dir()
>  > +        pool_dir = self._check_and_query_pool_dir(pool_dir)
>  >
>  >          while True:
>  >              hostname = self._query_hostname()
>  > @@ -75,7 +60,7 @@ class Wizard:
>  >              else:
>  >                  break
>  >
>  > -    def noninteractive(self, hostlist, pool_dir):
>  > +    def noninteractive(self, hostlist, pool_dir=None):
>
>  1. Why "None" and not "DefaultPoolDir" ?
>  
​Because when pool_dir is set to None, the loop will continue and user is
queried again.


>  2. Doesn't it make more sense to split this into it's own
> patch? It
>  doesn't look like it's logically connected to anything else changed in
>  this patch.
>  
​To split what in it's own patch?​


>  >          """ Starts Wizard in noninteractive mode
>  >          @param hostlist List of hosts (mandatory)
>  >          @param pool_dir Path to pool_directory (optional)
>  > @@ -330,13 +315,15 @@ class Wizard:
>  >                  sys.stderr.write("Hostname '%s' is not translatable
>  into a "
>  >                                   "valid IP address\n" % hostname)
>  >
>  > -    def _query_pool_dir(self):
>  > +    def _check_and_query_pool_dir(self, pool_dir):
>  >          """ Queries user for pool directory
>  > +        @param pool_dir Optional pool_dir which will be checked and
>  used if OK
>  >          @return Valid (is writable by user) path to directory
>  >          """
>  >          while True:
>  > -            pool_dir = raw_input("Enter path to a pool directory "
>  > -                                 "(default: '%s'): " %
> DefaultPoolDir)
>  > +            if pool_dir is None:
>  > +                pool_dir = raw_input("Enter path to a pool directory "
>  > +                                     "(default: '%s'): " %
>  DefaultPoolDir)
>  >              if pool_dir == "":
>  >                  pool_dir = DefaultPoolDir
>  >
>  > @@ -359,6 +346,7 @@ class Wizard:
>  >              elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
>  >                  sys.stderr.write("Directory '%s' is not
> writable\n"
>  >                                   % pool_dir)
>  > +            pool_dir = None
>  >
>  >      def _query_port(self):
>  >          """ Queries user for port
>  > --
>  > 2.4.3
>  >
>  > _______________________________________________
>  > LNST-developers mailing list
>  > LNST-developers(a)lists.fedorahosted.org
>  > https://lists.fedorahosted.org/mailman/listinfo/lnst-developers
>

[PATCH v2 2/8] pool-wizard: Rework and rename method _query_pool_dir to _check_and_query_pool_dir

[Ondrej Lichtner]

On Thu, Sep 03, 2015 at 10:36:49AM +0200, Jiri Prochazka wrote:
>  2015-09-02 15:18 GMT+02:00 Ondrej Lichtner
> <olichtne(a)redhat.com&gt;:
>  
>  > On Fri, Aug 28, 2015 at 11:17:18AM +0200, Jiri Prochazka wrote:
>  > > This patch introduces several changes to the _query_pool_dir method in
>  > lnst.Controller.Wizard:
>  > > - Method was renamed to _check_and_query_pool_dir to better express
>  > nature of the method
>  > > - Method now uses 1 optional argument - pool_dir - if set to something
>  > else than None,
>  > >   it checks if it's valid path to existing or creatable dir, if it is,
>  > uses it,
>  > >   else queries user for a new path (which is also checked)
>  > > - Method noninteractive has added default value (None) for argument
>  > pool_dir
>  > >
>  > > This method was reworked to prevent code duplication which would occur
>  > when mode for virtual
>  > > machines would be introduced
>  > >
>  > > Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  > > ---
>  > >  lnst/Controller/Wizard.py | 28 ++++++++--------------------
>  > >  1 file changed, 8 insertions(+), 20 deletions(-)
>  > >
>  > > diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
>  > > index 51856e8..b35b216 100644
>  > > --- a/lnst/Controller/Wizard.py
>  > > +++ b/lnst/Controller/Wizard.py
>  > > @@ -32,22 +32,7 @@ class Wizard:
>  > >          """ Starts Wizard in an interactive mode
>  > >          @param pool_dir Path to pool directory (optional)
>  > >          """
>  > > -        if pool_dir is None:
>  > > -            pool_dir = self._query_pool_dir()
>  > > -        else:
>  > > -            rv = self._check_path(pool_dir)
>  > > -            if rv == PATH_IS_DIR_ACCESSIBLE:
>  > > -                print("Pool directory set to '%s'" %
> pool_dir)
>  > > -            elif rv == PATH_DOES_NOT_EXIST:
>  > > -                sys.stderr.write("Path '%s' does not
> exist\n" %
>  > pool_dir)
>  > > -                pool_dir = self._query_pool_dir()
>  > > -            elif rv == PATH_NOT_DIR:
>  > > -                sys.stderr.write("Path '%s' exists but is not a
>  > directory\n"
>  > > -                                 % pool_dir)
>  > > -                pool_dir = self._query_pool_dir()
>  > > -            elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
>  > > -                sys.stderr.write("Directory '%s' is not
> writable\n" %
>  > pool_dir)
>  > > -                pool_dir = self._query_pool_dir()
>  > > +        pool_dir = self._check_and_query_pool_dir(pool_dir)
>  > >
>  > >          while True:
>  > >              hostname = self._query_hostname()
>  > > @@ -75,7 +60,7 @@ class Wizard:
>  > >              else:
>  > >                  break
>  > >
>  > > -    def noninteractive(self, hostlist, pool_dir):
>  > > +    def noninteractive(self, hostlist, pool_dir=None):
>  >
>  > 1. Why "None" and not "DefaultPoolDir" ?
>  >
>  ​Because when pool_dir is set to None, the loop will continue and user is
>  queried again. 
This is noninteractive mode... no user queries.

>  
>  
>  > 2. Doesn't it make more sense to split this into it's own patch? It
>  > doesn't look like it's logically connected to anything else changed in
>  > this patch.
>  >
>  ​To split what in it's own patch?​ 
This change of the default parameter, it doesn't look like it's
connected to any other changes this patch makes so shouldn't it be in
it's own patch?

>  
>  
>  >
>  > >          """ Starts Wizard in noninteractive mode
>  > >          @param hostlist List of hosts (mandatory)
>  > >          @param pool_dir Path to pool_directory (optional)
>  > > @@ -330,13 +315,15 @@ class Wizard:
>  > >                  sys.stderr.write("Hostname '%s' is not
> translatable
>  > into a "
>  > >                                   "valid IP address\n" % hostname)
>  > >
>  > > -    def _query_pool_dir(self):
>  > > +    def _check_and_query_pool_dir(self, pool_dir):
>  > >          """ Queries user for pool directory
>  > > +        @param pool_dir Optional pool_dir which will be checked and
>  > used if OK
>  > >          @return Valid (is writable by user) path to directory
>  > >          """
>  > >          while True:
>  > > -            pool_dir = raw_input("Enter path to a pool directory "
>  > > -                                 "(default: '%s'): " %
> DefaultPoolDir)
>  > > +            if pool_dir is None:
>  > > +                pool_dir = raw_input("Enter path to a pool directory
> "
>  > > +                                     "(default: '%s'): " %
>  > DefaultPoolDir)
>  > >              if pool_dir == "":
>  > >                  pool_dir = DefaultPoolDir
>  > >
>  > > @@ -359,6 +346,7 @@ class Wizard:
>  > >              elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
>  > >                  sys.stderr.write("Directory '%s' is not
> writable\n"
>  > >                                   % pool_dir)
>  > > +            pool_dir = None
>  > >
>  > >      def _query_port(self):
>  > >          """ Queries user for port
>  > > --
>  > > 2.4.3
>  > >
>  > > _______________________________________________
>  > > LNST-developers mailing list
>  > > LNST-developers(a)lists.fedorahosted.org
>  > > https://lists.fedorahosted.org/mailman/listinfo/lnst-developers
>  >

[PATCH v2 3/8] pool-wizard: Add write_to_xml method

[Jiri Prochazka]

The method tries to create a file and write in it contents of result XML,
if it success, returns True, else it returns False

This method was created to improve readability and modularity of the pool
wizard code.

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Wizard.py | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
index b35b216..e91413d 100644
--- a/lnst/Controller/Wizard.py
+++ b/lnst/Controller/Wizard.py
@@ -226,17 +226,20 @@ class Wizard:
             return
 
 
+    def _write_to_file(self, pool_dir, filename, doc):
+        """ Writes contents of XML to a file
+        @param pool_dir Path to directory where the file will be created
+        @param filename Name of the created file
+        @param doc Contents of XML file
+        @return True if file was successfuly written, False otherwise
+        """
         try:
             f = open(pool_dir + "/" + filename, "w")
             f.write(doc.toprettyxml())
             f.close()
+            return True
         except:
-            msg = "File '%s/%s' could not be opened or data
written\n"\
-                  % (pool_dir, filename)
-            sys.stderr.write(msg)
-            return
-
-        print("File '%s' successfuly created" % filename)
+            return False
 
     def _get_connection(self, hostname, port):
         """ Connects to machine
-- 
2.4.3

[PATCH v2 3/8] pool-wizard: Add write_to_xml method

[Ondrej Lichtner]

On Fri, Aug 28, 2015 at 11:17:19AM +0200, Jiri Prochazka wrote:
>  The method tries to create a file and write in it contents of result
> XML,
>  if it success, returns True, else it returns False
>  
>  This method was created to improve readability and modularity of the pool
>  wizard code. 
I'd say this patch should also update the create_xml method that this
was split from, instead of doing the update in a different patch.

>  
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  ---
>   lnst/Controller/Wizard.py | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
>  
>  diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
>  index b35b216..e91413d 100644
>  --- a/lnst/Controller/Wizard.py
>  +++ b/lnst/Controller/Wizard.py
>  @@ -226,17 +226,20 @@ class Wizard:
>               return
>   
>   
>  +    def _write_to_file(self, pool_dir, filename, doc):
>  +        """ Writes contents of XML to a file
>  +        @param pool_dir Path to directory where the file will be created
>  +        @param filename Name of the created file
>  +        @param doc Contents of XML file
>  +        @return True if file was successfuly written, False otherwise
>  +        """
>           try:
>               f = open(pool_dir + "/" + filename, "w")
>               f.write(doc.toprettyxml())
>               f.close()
>  +            return True
>           except:
>  -            msg = "File '%s/%s' could not be opened or data
> written\n"\
>  -                  % (pool_dir, filename)
>  -            sys.stderr.write(msg)
>  -            return
>  -
>  -        print("File '%s' successfuly created" % filename)
>  +            return False
>   
>       def _get_connection(self, hostname, port):
>           """ Connects to machine
>  -- 
>  2.4.3
>  
>  _______________________________________________
>  LNST-developers mailing list
>  LNST-developers(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/mailman/listinfo/lnst-developers

[PATCH v2 4/8] pool-wizard: Rework arguments and documentation for _create_xml method

[Jiri Prochazka]

This patch introduces several minor changes to _create_xml method:
  - Removes redundant argument "port"
  - Adds new argument "libvirt_domain" which will be used in virtual mode
  - Updates documentation for the method
  - Adds default values (None) to all arguments
  - Calls of the functions in interactive and noninteractive methods were
    adjusted to be up-to-date with the changes

Changes were made as preparation for implementing virtual mode, because
new virtual method will use _create_xml method differently than existing ones

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Wizard.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
index e91413d..1a3ecc0 100644
--- a/lnst/Controller/Wizard.py
+++ b/lnst/Controller/Wizard.py
@@ -53,8 +53,9 @@ class Wizard:
                                  "'%s:%s'\n" % (hostname, port))
             elif machine_interfaces is not None:
                 filename = self._query_filename(hostname)
-                self._create_xml(machine_interfaces, hostname,
-                                 port, pool_dir, filename, "interactive")
+                self._create_xml(machine_interfaces=machine_interfaces,
+                                 hostname=hostname, pool_dir=pool_dir,
+                                 filename=filename, mode="interactive")
             if self._query_continuation():
                 continue
             else:
@@ -131,8 +132,9 @@ class Wizard:
                 continue
             else:
                 filename = hostname + ".xml"
-                self._create_xml(machine_interfaces, hostname,
-                                 port, pool_dir, filename, "noninteractive")
+                self._create_xml(machine_interfaces=machine_interfaces,
+                                 hostname=hostname, pool_dir=pool_dir,
+                                 filename=filename, mode="noninteractive")
 
     def _check_hostname(self, hostname):
         """ Checks hostnames translatibility
@@ -174,15 +176,16 @@ class Wizard:
             sys.stderr.write("Failed creating dir\n")
             return None
 
-    def _create_xml(self, machine_interfaces, hostname,
-                    port, pool_dir, filename, mode):
+    def _create_xml(self, machine_interfaces=None, hostname=None,
+                    pool_dir=None, filename=None, mode=None,
+                    libvirt_domain=None):
         """ Creates slave machine XML file
         @param machine_interfaces Dictionary with machine's interfaces
         @param hostname Hostname of the machine
-        @param port Port on which LNST listens on the machine
         @param pool_dir Path to directory where XML file will be created
         @param filename Name of the XML file
         @param mode Mode in which wizard was started
+        @param libvirt_domain Libvirt domain of virtual host
         """
 
         impl = getDOMImplementation()
-- 
2.4.3

[PATCH v2 4/8] pool-wizard: Rework arguments and documentation for _create_xml method

[Ondrej Lichtner]

On Fri, Aug 28, 2015 at 11:17:20AM +0200, Jiri Prochazka wrote:
>  This patch introduces several minor changes to _create_xml method:
>    - Removes redundant argument "port" 
I'm guessing this was removed because it wasn't used? But then I see it
as a bug since we do have a slave machine param for the RPC port ->
"rpc_port", I see that in the wiki it's documented incorrectly as
"rpcport" so I'll fix that.

>    - Adds new argument "libvirt_domain" which will be used
> in virtual mode
>    - Updates documentation for the method
>    - Adds default values (None) to all arguments
>    - Calls of the functions in interactive and noninteractive methods were
>      adjusted to be up-to-date with the changes
>  
>  Changes were made as preparation for implementing virtual mode, because
>  new virtual method will use _create_xml method differently than existing ones
>  
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  ---
>   lnst/Controller/Wizard.py | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
>  
>  diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
>  index e91413d..1a3ecc0 100644
>  --- a/lnst/Controller/Wizard.py
>  +++ b/lnst/Controller/Wizard.py
>  @@ -53,8 +53,9 @@ class Wizard:
>                                    "'%s:%s'\n" % (hostname, port))
>               elif machine_interfaces is not None:
>                   filename = self._query_filename(hostname)
>  -                self._create_xml(machine_interfaces, hostname,
>  -                                 port, pool_dir, filename, "interactive")
>  +                self._create_xml(machine_interfaces=machine_interfaces,
>  +                                 hostname=hostname, pool_dir=pool_dir,
>  +                                 filename=filename, mode="interactive")
>               if self._query_continuation():
>                   continue
>               else:
>  @@ -131,8 +132,9 @@ class Wizard:
>                   continue
>               else:
>                   filename = hostname + ".xml"
>  -                self._create_xml(machine_interfaces, hostname,
>  -                                 port, pool_dir, filename, "noninteractive")
>  +                self._create_xml(machine_interfaces=machine_interfaces,
>  +                                 hostname=hostname, pool_dir=pool_dir,
>  +                                 filename=filename, mode="noninteractive")
>   
>       def _check_hostname(self, hostname):
>           """ Checks hostnames translatibility
>  @@ -174,15 +176,16 @@ class Wizard:
>               sys.stderr.write("Failed creating dir\n")
>               return None
>   
>  -    def _create_xml(self, machine_interfaces, hostname,
>  -                    port, pool_dir, filename, mode):
>  +    def _create_xml(self, machine_interfaces=None, hostname=None,
>  +                    pool_dir=None, filename=None, mode=None,
>  +                    libvirt_domain=None):
>           """ Creates slave machine XML file
>           @param machine_interfaces Dictionary with machine's interfaces
>           @param hostname Hostname of the machine
>  -        @param port Port on which LNST listens on the machine
>           @param pool_dir Path to directory where XML file will be created
>           @param filename Name of the XML file
>           @param mode Mode in which wizard was started
>  +        @param libvirt_domain Libvirt domain of virtual host
>           """
>   
>           impl = getDOMImplementation()
>  -- 
>  2.4.3
>  
>  _______________________________________________
>  LNST-developers mailing list
>  LNST-developers(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/mailman/listinfo/lnst-developers

[PATCH v2 4/8] pool-wizard: Rework arguments and documentation for _create_xml method

[Jiri Prochazka]

2015-09-02 15:32 GMT+02:00 Ondrej Lichtner <olichtne(a)redhat.com&gt;:

>  On Fri, Aug 28, 2015 at 11:17:20AM +0200, Jiri Prochazka wrote:
>  > This patch introduces several minor changes to _create_xml method:
>  >   - Removes redundant argument "port"
>
>  I'm guessing this was removed because it wasn't used? But then I see it
>  as a bug since we do have a slave machine param for the RPC port ->
>  "rpc_port", I see that in the wiki it's documented incorrectly as
>  "rpcport" so I'll fix that.
>
>  ​Yes, it wasn't used, but it would be handy to have it there as well, so I 
will
rework this again so the config contains the port as well​


>  >   - Adds new argument "libvirt_domain" which will be
> used in virtual mode
>  >   - Updates documentation for the method
>  >   - Adds default values (None) to all arguments
>  >   - Calls of the functions in interactive and noninteractive methods were
>  >     adjusted to be up-to-date with the changes
>  >
>  > Changes were made as preparation for implementing virtual mode, because
>  > new virtual method will use _create_xml method differently than existing
>  ones
>  >
>  > Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  > ---
>  >  lnst/Controller/Wizard.py | 17 ++++++++++-------
>  >  1 file changed, 10 insertions(+), 7 deletions(-)
>  >
>  > diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
>  > index e91413d..1a3ecc0 100644
>  > --- a/lnst/Controller/Wizard.py
>  > +++ b/lnst/Controller/Wizard.py
>  > @@ -53,8 +53,9 @@ class Wizard:
>  >                                   "'%s:%s'\n" % (hostname, port))
>  >              elif machine_interfaces is not None:
>  >                  filename = self._query_filename(hostname)
>  > -                self._create_xml(machine_interfaces, hostname,
>  > -                                 port, pool_dir, filename,
>  "interactive")
>  > +                self._create_xml(machine_interfaces=machine_interfaces,
>  > +                                 hostname=hostname, pool_dir=pool_dir,
>  > +                                 filename=filename, mode="interactive")
>  >              if self._query_continuation():
>  >                  continue
>  >              else:
>  > @@ -131,8 +132,9 @@ class Wizard:
>  >                  continue
>  >              else:
>  >                  filename = hostname + ".xml"
>  > -                self._create_xml(machine_interfaces, hostname,
>  > -                                 port, pool_dir, filename,
>  "noninteractive")
>  > +                self._create_xml(machine_interfaces=machine_interfaces,
>  > +                                 hostname=hostname, pool_dir=pool_dir,
>  > +                                 filename=filename,
>  mode="noninteractive")
>  >
>  >      def _check_hostname(self, hostname):
>  >          """ Checks hostnames translatibility
>  > @@ -174,15 +176,16 @@ class Wizard:
>  >              sys.stderr.write("Failed creating dir\n")
>  >              return None
>  >
>  > -    def _create_xml(self, machine_interfaces, hostname,
>  > -                    port, pool_dir, filename, mode):
>  > +    def _create_xml(self, machine_interfaces=None, hostname=None,
>  > +                    pool_dir=None, filename=None, mode=None,
>  > +                    libvirt_domain=None):
>  >          """ Creates slave machine XML file
>  >          @param machine_interfaces Dictionary with machine's interfaces
>  >          @param hostname Hostname of the machine
>  > -        @param port Port on which LNST listens on the machine
>  >          @param pool_dir Path to directory where XML file will be created
>  >          @param filename Name of the XML file
>  >          @param mode Mode in which wizard was started
>  > +        @param libvirt_domain Libvirt domain of virtual host
>  >          """
>  >
>  >          impl = getDOMImplementation()
>  > --
>  > 2.4.3
>  >
>  > _______________________________________________
>  > LNST-developers mailing list
>  > LNST-developers(a)lists.fedorahosted.org
>  > https://lists.fedorahosted.org/mailman/listinfo/lnst-developers
>

[PATCH v2 5/8] pool-wizard: Rework _create_xml method, add support for upcoming virtual mode

[Jiri Prochazka]

This patch prepares _create_xml method for support of virtual machines:
  - Adds support for virtual mode, which will write only hostname and
    libvirt domain to the output XML file
  - Changes the method so it uses newly created _write_to_xml method

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Wizard.py | 67 ++++++++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 30 deletions(-)

diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
index 1a3ecc0..570f0d1 100644
--- a/lnst/Controller/Wizard.py
+++ b/lnst/Controller/Wizard.py
@@ -197,37 +197,44 @@ class Wizard:
         param_el.setAttribute("name", "hostname")
         param_el.setAttribute("value", hostname)
         params_el.appendChild(param_el)
-        interfaces_el = doc.createElement("interfaces")
-        top_el.appendChild(interfaces_el)
-
-        interfaces_added = 0
-        for iface in machine_interfaces.itervalues():
-            if mode == "interactive":
-                answer = raw_input("Do you want to add interface '%s' (%s)
"
-                                   "to the recipe? [Y/n]: " %
(iface["name"],
-                                                              
iface["hwaddr"]))
-            if mode == "noninteractive" or answer.lower() == "y"\
-               or answer == "":
-                interfaces_added += 1
-                eth_el = doc.createElement("eth")
-                eth_el.setAttribute("id", iface["name"])
-                eth_el.setAttribute("label", "default_network")
-                interfaces_el.appendChild(eth_el)
-                params_el = doc.createElement("params")
-                eth_el.appendChild(params_el)
-                param_el = doc.createElement("param")
-                param_el.setAttribute("name", "hwaddr")
-                param_el.setAttribute("value", iface["hwaddr"])
-                params_el.appendChild(param_el)
-                param_el = doc.createElement("param")
-                param_el.setAttribute("name", "driver")
-                param_el.setAttribute("value", iface["driver"])
-                params_el.appendChild(param_el)
-        if interfaces_added == 0:
-            sys.stderr.write("You didn't add any interface, no file "
-                             "'%s' will be created\n" % filename)
-            return
+        if mode == "virtual":
+            param_el = doc.createElement("param")
+            param_el.setAttribute("name", "libvirt_domain")
+            param_el.setAttribute("value", libvirt_domain)
+            params_el.appendChild(param_el)
+        else:
+            interfaces_el = doc.createElement("interfaces")
+            top_el.appendChild(interfaces_el)
+
+            interfaces_added = 0
+            for iface in machine_interfaces.itervalues():
+                if mode == "interactive":
+                    msg = "Do you want to add interface '%s' (%s) to the
"\
+                          "recipe? [Y/n]: " % (iface["name"],
iface["hwaddr"])
+                    answer = raw_input(msg)
+                if mode == "noninteractive" or answer.lower() ==
"y"\
+                   or answer == "":
+                    interfaces_added += 1
+                    eth_el = doc.createElement("eth")
+                    eth_el.setAttribute("id", iface["name"])
+                    eth_el.setAttribute("label", "default_network")
+                    interfaces_el.appendChild(eth_el)
+                    params_el = doc.createElement("params")
+                    eth_el.appendChild(params_el)
+                    param_el = doc.createElement("param")
+                    param_el.setAttribute("name", "hwaddr")
+                    param_el.setAttribute("value", iface["hwaddr"])
+                    params_el.appendChild(param_el)
+            if interfaces_added == 0:
+                sys.stderr.write("You didn't add any interface, no file "
+                                 "'%s' will be created\n" % filename)
+                return
 
+        if self._write_to_file(pool_dir, filename, doc):
+            print("File '%s/%s' successfuly created." % (pool_dir,
filename))
+        else:
+            sys.stderr.write("File '%s/%s' could not be opened "
+                             "or data written.\n" % (pool_dir, filename))
 
     def _write_to_file(self, pool_dir, filename, doc):
         """ Writes contents of XML to a file
-- 
2.4.3

[PATCH v2 6/8] pool-wizard: Add import libvirt and _query_libvirt_domain method

[Jiri Prochazka]

This patch adds import of libvirt library
This patch adds new method _query_libvirt_domain:
  - This method needs imported libvirt library to work
  - It tries to connect to hypervisor, if it succeeds, it checks if
    guest with entered name exists and tries to recover it's IP
    from DHCP lease in "default" network. If it fails, it queries
    user for new libvirt domain, otherwise it returns the string with
    the domain

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Wizard.py | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
index 570f0d1..137e758 100644
--- a/lnst/Controller/Wizard.py
+++ b/lnst/Controller/Wizard.py
@@ -10,6 +10,7 @@ __author__ = """
 jprochaz(a)redhat.com (Jiri Prochazka)
 """
 
+import libvirt
 import socket
 import sys
 import time
@@ -27,6 +28,9 @@ PATH_NOT_DIR = 3
 
 
 class Wizard:
+    def __init__(self):
+        # used for storing libvirt connection
+        self._conn = None
 
     def interactive(self, pool_dir=None):
         """ Starts Wizard in an interactive mode
@@ -375,3 +379,39 @@ class Wizard:
                     return port
                 except:
                     sys.stderr.write("Invalid port entered\n")
+
+    def _query_libvirt_domain(self):
+        """ Queries user for libvirt_domain
+        @note Virtual host must be running under libvirt
+              and has to have an IP from "default" network
+              DHCP server
+        @return String representing libvirt_domain of host
+        """
+        if self._conn is None:
+            self._conn = libvirt.openReadOnly("qemu:///system")
+
+        if self._conn is None:
+            sys.stderr.write("Failed to open connection to hypervisor\n")
+            return (None, None)
+
+        while True:
+            libvirt_domain = raw_input("Enter libvirt domain of virtual host:
")
+            if libvirt_domain == "":
+                sys.stderr.write("No domain entered\n")
+                continue
+            try:
+                self._conn.lookupByName(libvirt_domain)
+            except:
+                continue
+
+            for lease in
self._conn.networkLookupByName("default").DHCPLeases():
+                if lease["hostname"] == libvirt_domain:
+                    return (lease["ipaddr"], libvirt_domain)
+
+            sys.stderr.write("Couldn't find any IP associated with selected
libvirt_domain\n")
+            answer = raw_input("Do you want to add hostname manually? [Y/n]:
")
+            if answer.lower() == "y" or answer == "":
+                hostname = self._query_hostname()
+                return libvirt_domain, hostname
+            else:
+                continue
-- 
2.4.3

[PATCH v2 6/8] pool-wizard: Add import libvirt and _query_libvirt_domain method

[Jan Tluka]

Fri, Aug 28, 2015 at 11:17:22AM CEST, jprochaz(a)redhat.com wrote:
> This patch adds import of libvirt library
> This patch adds new method _query_libvirt_domain:
>   - This method needs imported libvirt library to work
>   - It tries to connect to hypervisor, if it succeeds, it checks if
>     guest with entered name exists and tries to recover it's IP
>     from DHCP lease in "default" network. If it fails, it queries
>     user for new libvirt domain, otherwise it returns the string with
>     the domain
>
> Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> ---
>  lnst/Controller/Wizard.py | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
> index 570f0d1..137e758 100644
> --- a/lnst/Controller/Wizard.py
> +++ b/lnst/Controller/Wizard.py
> @@ -10,6 +10,7 @@ __author__ = """
>  jprochaz(a)redhat.com (Jiri Prochazka)
>  """
>  
> +import libvirt 
Make this import conditional please, there might be users that do
not have the module available.

>  import socket
>  import sys
>  import time
> @@ -27,6 +28,9 @@ PATH_NOT_DIR = 3
>  
>  
>  class Wizard:
> +    def __init__(self):
> +        # used for storing libvirt connection
> +        self._conn = None
>  
>      def interactive(self, pool_dir=None):
>          """ Starts Wizard in an interactive mode
> @@ -375,3 +379,39 @@ class Wizard:
>                      return port
>                  except:
>                      sys.stderr.write("Invalid port entered\n")
> +
> +    def _query_libvirt_domain(self):
> +        """ Queries user for libvirt_domain
> +        @note Virtual host must be running under libvirt
> +              and has to have an IP from "default" network
> +              DHCP server
> +        @return String representing libvirt_domain of host
> +        """
> +        if self._conn is None:
> +            self._conn = libvirt.openReadOnly("qemu:///system")
> +
> +        if self._conn is None:
> +            sys.stderr.write("Failed to open connection to hypervisor\n")
> +            return (None, None) 
Can you move this code one level up to virtual()? It looks a bit weird
here.

> +
> +        while True:
> +            libvirt_domain = raw_input("Enter libvirt domain of virtual host:
> ")
> +            if libvirt_domain == "":
> +                sys.stderr.write("No domain entered\n")
> +                continue
> +            try:
> +                self._conn.lookupByName(libvirt_domain)
> +            except:
> +                continue 
                  ^^^ Please be friendly to user, print at least something,
ideally exception name but anything simple would work, too.

> +
> +            for lease in
> self._conn.networkLookupByName("default").DHCPLeases():
> +                if lease["hostname"] == libvirt_domain:
> +                    return (lease["ipaddr"], libvirt_domain)
> +
> +            sys.stderr.write("Couldn't find any IP associated with selected
> libvirt_domain\n") 
Which libvirt_domain? Please write the value of libvirt_domain. 

> +            answer = raw_input("Do you want to add hostname
> manually? [Y/n]: ")
> +            if answer.lower() == "y" or answer == "":
> +                hostname = self._query_hostname()
> +                return libvirt_domain, hostname
> +            else:
> +                continue 
I don't get it. So if I do not answer Y to the question I will be
silently ignored? I'd rather expect to be asked to enter the hostname directly
and not whether I want to add it manually. Is there option to have it
automagically? I think the question is unnecessary here.

> -- 
> 2.4.3
>
> _______________________________________________
> LNST-developers mailing list
> LNST-developers(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/lnst-developers

[PATCH v2 6/8] pool-wizard: Add import libvirt and _query_libvirt_domain method

[Jiri Prochazka]

2015-09-01 17:30 GMT+02:00 Jan Tluka <jtluka(a)redhat.com&gt;:

>  Fri, Aug 28, 2015 at 11:17:22AM CEST, jprochaz(a)redhat.com wrote:
>  >This patch adds import of libvirt library
>  >This patch adds new method _query_libvirt_domain:
>  >  - This method needs imported libvirt library to work
>  >  - It tries to connect to hypervisor, if it succeeds, it checks if
>  >    guest with entered name exists and tries to recover it's IP
>  >    from DHCP lease in "default" network. If it fails, it queries
>  >    user for new libvirt domain, otherwise it returns the string with
>  >    the domain
>  >
>  >Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  >---
>  > lnst/Controller/Wizard.py | 40 ++++++++++++++++++++++++++++++++++++++++
>  > 1 file changed, 40 insertions(+)
>  >
>  >diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
>  >index 570f0d1..137e758 100644
>  >--- a/lnst/Controller/Wizard.py
>  >+++ b/lnst/Controller/Wizard.py
>  >@@ -10,6 +10,7 @@ __author__ = """
>  > jprochaz(a)redhat.com (Jiri Prochazka)
>  > """
>  >
>  >+import libvirt
>
>  Make this import conditional please, there might be users that do
>  not have the module available.
>
>  > import socket
>  > import sys
>  > import time
>  >@@ -27,6 +28,9 @@ PATH_NOT_DIR = 3
>  >
>  >
>  > class Wizard:
>  >+    def __init__(self):
>  >+        # used for storing libvirt connection
>  >+        self._conn = None
>  >
>  >     def interactive(self, pool_dir=None):
>  >         """ Starts Wizard in an interactive mode
>  >@@ -375,3 +379,39 @@ class Wizard:
>  >                     return port
>  >                 except:
>  >                     sys.stderr.write("Invalid port entered\n")
>  >+
>  >+    def _query_libvirt_domain(self):
>  >+        """ Queries user for libvirt_domain
>  >+        @note Virtual host must be running under libvirt
>  >+              and has to have an IP from "default" network
>  >+              DHCP server
>  >+        @return String representing libvirt_domain of host
>  >+        """
>  >+        if self._conn is None:
>  >+            self._conn = libvirt.openReadOnly("qemu:///system")
>  >+
>  >+        if self._conn is None:
>  >+            sys.stderr.write("Failed to open connection to hypervisor\n")
>  >+            return (None, None)
>
>  Can you move this code one level up to virtual()? It looks a bit weird
>  here.
>
>  >+
>  >+        while True:
>  >+            libvirt_domain = raw_input("Enter libvirt domain of virtual
>  host: ")
>  >+            if libvirt_domain == "":
>  >+                sys.stderr.write("No domain entered\n")
>  >+                continue
>  >+            try:
>  >+                self._conn.lookupByName(libvirt_domain)
>  >+            except:
>  >+                continue
>
>                    ^^^ Please be friendly to user, print at least something,
>  ideally exception name but anything simple would work, too.
>  
​The libvirt itself prints error message for user - ​

​"libvirt: QEMU Driver error : Domain not found: no domain with matching
name 'xyz'"​

>  >+
>  >+            for lease in
>  self._conn.networkLookupByName("default").DHCPLeases():
>  >+                if lease["hostname"] == libvirt_domain:
>  >+                    return (lease["ipaddr"], libvirt_domain)
>  >+
>  >+            sys.stderr.write("Couldn't find any IP associated with
>  selected libvirt_domain\n")
>
>  Which libvirt_domain? Please write the value of libvirt_domain.
>
>  >+            answer = raw_input("Do you want to add hostname manually?
>  [Y/n]: ")
>  >+            if answer.lower() == "y" or answer == "":
>  >+                hostname = self._query_hostname()
>  >+                return libvirt_domain, hostname
>  >+            else:
>  >+                continue
>
>  I don't get it. So if I do not answer Y to the question I will be
>  silently ignored? I'd rather expect to be asked to enter the hostname
>  directly
>  and not whether I want to add it manually. Is there option to have it
>  automagically? I think the question is unnecessary here.
>
>  >--
>  >2.4.3
>  >
>  >_______________________________________________
>  >LNST-developers mailing list
>  &gt;LNST-developers(a)lists.fedorahosted.org
>  >https://lists.fedorahosted.org/mailman/listinfo/lnst-developers

[PATCH v2 6/8] pool-wizard: Add import libvirt and _query_libvirt_domain method

[Jan Tluka]

Thu, Sep 03, 2015 at 11:35:34AM CEST, jprochaz(a)redhat.com wrote:
> 2015-09-01 17:30 GMT+02:00 Jan Tluka <jtluka(a)redhat.com&gt;:
>
> > Fri, Aug 28, 2015 at 11:17:22AM CEST, jprochaz(a)redhat.com wrote:
> > >This patch adds import of libvirt library
> > >This patch adds new method _query_libvirt_domain:
> > >  - This method needs imported libvirt library to work
> > >  - It tries to connect to hypervisor, if it succeeds, it checks if
> > >    guest with entered name exists and tries to recover it's IP
> > >    from DHCP lease in "default" network. If it fails, it queries
> > >    user for new libvirt domain, otherwise it returns the string with
> > >    the domain
> > >
> > >Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> > >---
> > > lnst/Controller/Wizard.py | 40 ++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 40 insertions(+)
> > >
> > >diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
> > >index 570f0d1..137e758 100644
> > >--- a/lnst/Controller/Wizard.py
> > >+++ b/lnst/Controller/Wizard.py
> > >@@ -10,6 +10,7 @@ __author__ = """
> > > jprochaz(a)redhat.com (Jiri Prochazka)
> > > """
> > >
> > >+import libvirt
> >
> > Make this import conditional please, there might be users that do
> > not have the module available.
> >
> > > import socket
> > > import sys
> > > import time
> > >@@ -27,6 +28,9 @@ PATH_NOT_DIR = 3
> > >
> > >
> > > class Wizard:
> > >+    def __init__(self):
> > >+        # used for storing libvirt connection
> > >+        self._conn = None
> > >
> > >     def interactive(self, pool_dir=None):
> > >         """ Starts Wizard in an interactive mode
> > >@@ -375,3 +379,39 @@ class Wizard:
> > >                     return port
> > >                 except:
> > >                     sys.stderr.write("Invalid port entered\n")
> > >+
> > >+    def _query_libvirt_domain(self):
> > >+        """ Queries user for libvirt_domain
> > >+        @note Virtual host must be running under libvirt
> > >+              and has to have an IP from "default" network
> > >+              DHCP server
> > >+        @return String representing libvirt_domain of host
> > >+        """
> > >+        if self._conn is None:
> > >+            self._conn = libvirt.openReadOnly("qemu:///system")
> > >+
> > >+        if self._conn is None:
> > >+            sys.stderr.write("Failed to open connection to
> hypervisor\n")
> > >+            return (None, None)
> >
> > Can you move this code one level up to virtual()? It looks a bit weird
> > here.
> >
> > >+
> > >+        while True:
> > >+            libvirt_domain = raw_input("Enter libvirt domain of virtual
> > host: ")
> > >+            if libvirt_domain == "":
> > >+                sys.stderr.write("No domain entered\n")
> > >+                continue
> > >+            try:
> > >+                self._conn.lookupByName(libvirt_domain)
> > >+            except:
> > >+                continue
> >
> >                   ^^^ Please be friendly to user, print at least something,
> > ideally exception name but anything simple would work, too.
> >
> ​The libvirt itself prints error message for user - ​
>
> ​"libvirt: QEMU Driver error : Domain not found: no domain with matching
> name 'xyz'"​
>  
Ok, no big deal then.

> >
> > >+
> > >+            for lease in
> > self._conn.networkLookupByName("default").DHCPLeases():
> > >+                if lease["hostname"] == libvirt_domain:
> > >+                    return (lease["ipaddr"], libvirt_domain)
> > >+
> > >+            sys.stderr.write("Couldn't find any IP associated with
> > selected libvirt_domain\n")
> >
> > Which libvirt_domain? Please write the value of libvirt_domain.
> >
> > >+            answer = raw_input("Do you want to add hostname manually?
> > [Y/n]: ")
> > >+            if answer.lower() == "y" or answer == "":
> > >+                hostname = self._query_hostname()
> > >+                return libvirt_domain, hostname
> > >+            else:
> > >+                continue
> >
> > I don't get it. So if I do not answer Y to the question I will be
> > silently ignored? I'd rather expect to be asked to enter the hostname
> > directly
> > and not whether I want to add it manually. Is there option to have it
> > automagically? I think the question is unnecessary here.
> >
> > >--
> > >2.4.3
> > >
> > >_______________________________________________
> > >LNST-developers mailing list
> > &gt;LNST-developers(a)lists.fedorahosted.org
> > >https://lists.fedorahosted.org/mailman/listinfo/lnst-developers

[PATCH v2 6/8] pool-wizard: Add import libvirt and _query_libvirt_domain method

[Jan Tluka]

Fri, Aug 28, 2015 at 11:17:22AM CEST, jprochaz(a)redhat.com wrote:
> This patch adds import of libvirt library
> This patch adds new method _query_libvirt_domain:
>   - This method needs imported libvirt library to work
>   - It tries to connect to hypervisor, if it succeeds, it checks if
>     guest with entered name exists and tries to recover it's IP
>     from DHCP lease in "default" network. If it fails, it queries
>     user for new libvirt domain, otherwise it returns the string with
>     the domain
>
> Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> ---
>  lnst/Controller/Wizard.py | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
> index 570f0d1..137e758 100644
> --- a/lnst/Controller/Wizard.py
> +++ b/lnst/Controller/Wizard.py
> @@ -10,6 +10,7 @@ __author__ = """
>  jprochaz(a)redhat.com (Jiri Prochazka)
>  """
>  
> +import libvirt
>  import socket
>  import sys
>  import time
> @@ -27,6 +28,9 @@ PATH_NOT_DIR = 3
>  
>  
>  class Wizard:
> +    def __init__(self):
> +        # used for storing libvirt connection
> +        self._conn = None
>  
>      def interactive(self, pool_dir=None):
>          """ Starts Wizard in an interactive mode
> @@ -375,3 +379,39 @@ class Wizard:
>                      return port
>                  except:
>                      sys.stderr.write("Invalid port entered\n")
> +
> +    def _query_libvirt_domain(self):
> +        """ Queries user for libvirt_domain
> +        @note Virtual host must be running under libvirt
> +              and has to have an IP from "default" network
> +              DHCP server
> +        @return String representing libvirt_domain of host
> +        """
> +        if self._conn is None:
> +            self._conn = libvirt.openReadOnly("qemu:///system")
> +
> +        if self._conn is None:
> +            sys.stderr.write("Failed to open connection to hypervisor\n")
> +            return (None, None)
> +
> +        while True:
> +            libvirt_domain = raw_input("Enter libvirt domain of virtual host:
> ")
> +            if libvirt_domain == "":
> +                sys.stderr.write("No domain entered\n")
> +                continue
> +            try:
> +                self._conn.lookupByName(libvirt_domain)
> +            except:
> +                continue
> +
> +            for lease in
> self._conn.networkLookupByName("default").DHCPLeases():
> +                if lease["hostname"] == libvirt_domain:
> +                    return (lease["ipaddr"], libvirt_domain) 
Here you return 1. hostname/ip_address 2. domain

> +
> +            sys.stderr.write("Couldn't find any IP associated with selected
> libvirt_domain\n")
> +            answer = raw_input("Do you want to add hostname manually? [Y/n]:
> ")
> +            if answer.lower() == "y" or answer == "":
> +                hostname = self._query_hostname()
> +                return libvirt_domain, hostname 
Here you return 1. domain 2. hostname/ip_address. I think this is a bug.

> +            else:
> +                continue
> -- 
> 2.4.3
>
> _______________________________________________
> LNST-developers mailing list
> LNST-developers(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/lnst-developers

[PATCH v2 7/8] pool-wizard: Add virtual method

[Jiri Prochazka]

This patch adds method for adding virtual machines via pool wizard:
  - Method queries user for pool_dir andlibvirt_domain, if valid ones are entered and
    successfuly checked, it creates XML file containing hostname and libvirt_domain
    needed for valid slave virtual machine config

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Wizard.py | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
index 137e758..a4e5772 100644
--- a/lnst/Controller/Wizard.py
+++ b/lnst/Controller/Wizard.py
@@ -15,7 +15,7 @@ import socket
 import sys
 import time
 import os
-from lnst.Common.Utils import mkdir_p
+from lnst.Common.Utils import mkdir_p, check_process_running
 from lnst.Common.Config import DefaultRPCPort
 from lnst.Common.ConnectionHandler import send_data, recv_data
 from xml.dom.minidom import getDOMImplementation
@@ -140,6 +140,38 @@ class Wizard:
                                  hostname=hostname, pool_dir=pool_dir,
                                  filename=filename, mode="noninteractive")
 
+    def virtual(self, pool_dir=None):
+        """ Starts Wizard in a virtual mode
+        @param pool_dir Path to pool directory (optional)
+        """
+
+        print("WARNING: For LNST Pool Wizard to work with virtual guests, "\
+               "several conditions have to be met: \n"\
+               "\t1) Guests must be running under libvirt and QEMU\n"\
+               "\t2) Guests must be in \"default\" network and have an IP
"\
+               "address from DHCP in that network")
+
+        if not check_process_running("libvirtd"):
+            sys.stderr.write("libvirtd is not running, aborting wizard\n")
+            return
+
+        pool_dir = self._check_and_query_pool_dir(pool_dir)
+
+        while True:
+            hostname, libvirt_domain= self._query_libvirt_domain()
+            if hostname is None:
+                return
+            filename = self._query_filename(libvirt_domain)
+
+            self._create_xml(hostname=hostname, pool_dir=pool_dir,
+                             filename=filename, mode="virtual",
+                             libvirt_domain=libvirt_domain)
+
+            if self._query_continuation():
+                continue
+            else:
+                break
+
     def _check_hostname(self, hostname):
         """ Checks hostnames translatibility
         @param hostname Hostname which is checked whether it's valid
-- 
2.4.3

[PATCH v2 8/8] pool-wizard: Add support for virtual mode in LNST Pool Wizard

[Jiri Prochazka]

Users now can use the pool wizard for adding virtual machines running
under libvirt and QEMU.

To use wizard in the VM mode, use option -v or --virtual. The option is
compatible with -p and --pool-dir option

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst-pool-wizard | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lnst-pool-wizard b/lnst-pool-wizard
index 9da8911..a1ed03f 100755
--- a/lnst-pool-wizard
+++ b/lnst-pool-wizard
@@ -29,6 +29,7 @@ def help(retval=0):
           " -i, --interactive          start wizard in interactive mode (this
"\
           "is default mode)\n"\
           " -n, --noninteractive       start wizard in noninteractive mode\n"\
+          " -v, --virtual              start wizard in mode for VMs\n"\
           "Examples:\n"\
           " lnst-pool-wizard --interactive\n"\
           " lnst-pool-wizard --noninteractive 192.168.122.2\n"\
@@ -41,8 +42,8 @@ def main():
     try:
         opts, args = getopt.getopt(
             sys.argv[1:],
-            "hinp:",
-            ["help", "interactive", "noninteractive",
"pool_dir="]
+            "hinvp:",
+            ["help", "interactive", "noninteractive",
"virtual", "pool_dir="]
         )
     except getopt.GetoptError as err:
         sys.stderr.write(str(err))
@@ -63,6 +64,8 @@ def main():
             else:
                 hostlist = args
             mode = "noninteractive"
+        elif opt in ("-v", "--virtual"):
+            mode = "virtual"
         elif opt in ("-p", "--pool_dir"):
             if not arg:
                 sys.stderr.write("No pool directory specified\n")
@@ -75,6 +78,8 @@ def main():
 
     if mode == "noninteractive":
         w.noninteractive(hostlist, pool_dir)
+    elif mode == "virtual":
+        w.virtual(pool_dir)
     else:
         w.interactive(pool_dir)
 
-- 
2.4.3