On Sun, Jun 16, 2013 at 11:28 PM, Dimitris Aragiorgis <[email protected]>wrote:

> Hi Thomas,
>
> * Thomas Thrainer <[email protected]> [2013-06-12 15:55:20 +0200]:
>
> > Hi Dimitris,
>
> >
> > I was trying to apply your patch regarding NIC configuration scripts.
> After
> > having rebased them to current master and fixed a problem with
> distcheck, I
> > run into the following problem when creating and instance:
> > ...
> > Failure: command execution error:
> > Could not start instance: Error while executing backend function: Cannot
> > create needed directory '/var/run/ganeti/xen-hypervisor/nic/
> > instance.example.com': [Errno 2] No such file or directory:
> > '/var/run/ganeti/xen-hypervisor/nic/instance.example.com'
> >
> > ...
> > GenericError: Cannot create needed directory
> > '/var/run/ganeti/xen-hypervisor/nic/instance.example.com': [Errno 2] No
> > such file or directory: '/var/run/ganeti/xen-hypervisor/nic/
> > instance.example.com'
> >
> > And indeed, there is no /var/run/ganeti/xen-hypervisor/nic/ directory on
> > that node. Those directories should be created as well, just like the KVM
> > version does in hv_kvm.py:600.
> >
> >
>
> sorry for the late reply. If I understand correctly the following interdiff
> should fix the issue:
>
> diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
> index b7f998c..99ad649 100644
> --- a/lib/hypervisor/hv_xen.py
> +++ b/lib/hypervisor/hv_xen.py
> @@ -316,7 +316,8 @@ class XenHypervisor(hv_base.BaseHypervisor):
>    REBOOT_RETRY_COUNT = 60
>    REBOOT_RETRY_INTERVAL = 10
>    _ROOT_DIR = pathutils.RUN_DIR + "/xen-hypervisor"
> -  _NICS_DIR = _ROOT_DIR + "/nic" # contains instances nic <-> tap
> associations
> +  _NICS_DIR = _ROOT_DIR + "/nic" # contains NICs' info
> +  _DIRS = [_ROOT_DIR, _NICS_DIR]
>
>    ANCILLARY_FILES = [
>      XEND_CONFIG_FILE,
> @@ -342,6 +343,9 @@ class XenHypervisor(hv_base.BaseHypervisor):
>
>      self._cmd = _cmd
>
> +    dirs = [(dname, constants.RUN_DIRS_MODE) for dname in self._DIRS]
> +    utils.EnsureDirs(dirs)
> +
>    def _GetCommand(self, hvparams):
>      """Returns Xen command to use.
>
> Still it does not. Python unittests fail in EnsureDirs with:
> GenericError: Cannot create needed directory
> '/usr/local/var/run/ganeti/xen-hypervisor': [Errno 2] No such file or
> directory: '/usr/local/var/run/ganeti/xen-hypervisor'
>
> Any idea why? What should we do?
> BTW why not change the behavior of EnsureDirs and make it like mkdir -p ?
>

The problem is, that the XenHypervisor constructor is run during the tests.
You could just create the required parent directories when you create the
instance directory as well. What about the following interdiff?:

diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
index dd423ff..a1a180f 100644
--- a/lib/hypervisor/hv_xen.py
+++ b/lib/hypervisor/hv_xen.py
@@ -317,7 +317,8 @@ class XenHypervisor(hv_base.BaseHypervisor):
   REBOOT_RETRY_COUNT = 60
   REBOOT_RETRY_INTERVAL = 10
   _ROOT_DIR = pathutils.RUN_DIR + "/xen-hypervisor"
-  _NICS_DIR = _ROOT_DIR + "/nic" # contains instances nic <-> tap
associations
+  _NICS_DIR = _ROOT_DIR + "/nic" # contains NICs' info
+  _DIRS = [_ROOT_DIR, _NICS_DIR]

   ANCILLARY_FILES = [
     XEND_CONFIG_FILE,
@@ -394,8 +395,10 @@ class XenHypervisor(hv_base.BaseHypervisor):
     This version of the function just writes the config file from static
data.

     """
-    utils.EnsureDirs([(cls._InstanceNICDir(instance_name),
-                     constants.RUN_DIRS_MODE)])
+    dirs = [(dname, constants.RUN_DIRS_MODE)
+            for dname in cls._DIRS + cls._InstanceNICDir(instance_name)]
+    utils.EnsureDirs(dirs)
+
     cfg_file = cls._InstanceNICFile(instance_name, idx)
     data = StringIO()




>
> > Also, there might be a better solution to the distcheck problem. The root
> > is that we write to a path outside of $PREFIX during `make install`
> (namely
> > to /etc/xen/, or more precisely to $XEN_CONFIG_DIR). Actually we
> shouldn't
> > be doing that, but only provide documentation to the user what he has to
> do
> > himself after the installation. I agree that that's not as convenient as
> > having `make install` do it for him...
> >
>
> True :). So the install-exec-hook is not what you want. You prefer to let
> the script in /usr/lib/ganeti/ and just document that the user should
> create the appropriate link in order to use it? That's OK by me.
>
>
I'd like to hear Guido's opinion on that as well, as he originally
suggested the inclusion as install-exec-hook. He's back from vacation as of
today, so probably he's able to comment on this soonish.


> So, as soon as we address the NICs' dir issue, we are OK right?
> BTW Is there any reason why we push it on master
> and not on stable-2.8? I think it is more like a "bug" fix than an feature.
> Passing mode=routed and get a bridged NIC without any warning is not wanted
> right?
>

The reason why I wanted to push it to master is because the patch title
says so :). If you'd prefer to have it in 2.8 as well (which sounds
reasonable), please rebase it and resend the patches with [PATCH
stable-2.8] as subject prefix.


>
> > I attach the modified patches, so you don't have to rebase them again.
> >
> > Thanks,
> > Thomas
>
> Thanks a lot,
> dimara
>

Thanks,
Thomas

>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iQEcBAEBCAAGBQJRvi4CAAoJEMk10sCdtK5ja5sH/jqYbQE4JUmkbeIOdg9akKc/
> hGZUPpehMdP21ikMkQkUEHN4DLWc+7OhuwDDj0L5yXl0Wm8U2EbtZvwmaFLTdAgp
> oJbyWWFLkY2tRtkIVUfj4pUjfVrKtymZGmXa4+X4DyjIk8BlWHaY/+rPUoa0vqhk
> C1FdXk8JqDuqGlS6BRWXxN9pbuGWOVOhazP1Zy3NoK+Wpu9hZ8xEYOLf9g3zOgwW
> I1jyyhKgPbO5MUOLgF53LZ+mx3KatX6NQEDRzE0wARfD2Yc8t18XfKWKj9LQjY2m
> oghkztx3wxesgNR7rZPUnyPzhrvsDsQAfi1akNiOISHPEeCd8jXYt0UcAH265xI=
> =7fXW
> -----END PGP SIGNATURE-----
>
>


-- 
Thomas Thrainer | Software Engineer | [email protected] |

Google Germany GmbH
Dienerstr. 12
80331 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Katherine Stephens

Reply via email to