On Mon, Jun 17, 2013 at 12:08 PM, Dimitris Aragiorgis <[email protected]> wrote: > Hi, > > * Thomas Thrainer <[email protected]> [2013-06-17 09:47:35 +0200]: > >> On Sun, Jun 16, 2013 at 11:28 PM, Dimitris Aragiorgis <[email protected]>wrote: >> >> >> 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() >> >> >> >> > > I see. This fixes the issue then. Thanks. > >> > >> > > 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. >>
Why is it a bug fix? Which bug does it fix? >> > > OK. I like install-exec-hook workaround too, but we'll wait for Guido's > opinion. > Until then I'll rebase the patch on top of stable-2.8 in case we want to use > it. > Ok, let's just document that the link is needed, for now. But we should also verify it in cluster verify, for example. Thanks, Guido
