LGTM, thanks On Thu, Jul 30, 2015 at 9:56 PM, Dimitris Aragiorgis < [email protected]> wrote:
> Until now all implementations at the bdev level require a local > block device on the primary node. Since there are storage > technologies that allow userspace access only without the need of > a block device, (e.g. QEMU + RADOS), this patch makes the > corresponding changes to support this kind of functionality. > Note that for this to work you need corresponding support in the > OS definition. > > This patch changes the backend logic so that it can handle the > absence of local block devices. > > Finally, it extends the ExtStorage interface to support the above > functionality. Until now the 'attach' script returned the block > device in the first line. With this patch, if this line is empty, > it denotes that a local block device is not available. > > An example could be a RADOS provider where the volume does not get > mapped locally (i.e., no /dev/rbdX device) and the attach script > returns two lines: an empty line denoting that no local block device > exists and a second line with the appropriate KVM userspace URI > (i.e. kvm:rbd:<pool>/<volume name>). > > Adjust OpInstanceActivateDisks so that a None dev_path is handled > correctly as a return value. > > Signed-off-by: Dimitris Aragiorgis <[email protected]> > --- > doc/design-shared-storage.rst | 4 +++- > lib/backend.py | 7 ++++++- > lib/storage/extstorage.py | 10 ++++++++++ > man/ganeti-extstorage-interface.rst | 17 +++++++++++++++++ > src/Ganeti/OpCodes.hs | 2 +- > 5 files changed, 37 insertions(+), 3 deletions(-) > > diff --git a/doc/design-shared-storage.rst b/doc/design-shared-storage.rst > index d389e03..0390264 100644 > --- a/doc/design-shared-storage.rst > +++ b/doc/design-shared-storage.rst > @@ -329,7 +329,9 @@ with the hypervisor it corresponds to (e.g. > kvm:<uri>). The prefix will > be case insensitive. If the 'attach' script doesn't return any extra > lines, we assume that the ExtStorage provider doesn't support userspace > access (this way we maintain backward compatibility with the existing > -'attach' scripts). > +'attach' scripts). In case the provider supports *only* userspace > +access and thus a local block device is not available, then the first > +line should be an empty line. > > The 'GetUserspaceAccessUri' method of the 'ExtStorageDevice' class will > parse the output of the 'attach' script and if the provider supports > diff --git a/lib/backend.py b/lib/backend.py > index 140c3cc..4674002 100644 > --- a/lib/backend.py > +++ b/lib/backend.py > @@ -2538,6 +2538,10 @@ def _SymlinkBlockDev(instance_name, device_path, > idx): > @return: absolute path to the disk's symlink > > """ > + # In case we have only a userspace access URI, device_path is None > + if not device_path: > + return None > + > link_name = _GetBlockDevSymlinkPath(instance_name, idx) > try: > os.symlink(device_path, link_name) > @@ -4045,9 +4049,10 @@ def OSEnvironment(instance, inst_os, debug=0): > for idx, disk in enumerate(instance.disks_info): > real_disk = _OpenRealBD(disk) > uri = _CalculateDeviceURI(instance, disk, real_disk) > - result["DISK_%d_PATH" % idx] = real_disk.dev_path > result["DISK_%d_ACCESS" % idx] = disk.mode > result["DISK_%d_UUID" % idx] = disk.uuid > + if real_disk.dev_path: > + result["DISK_%d_PATH" % idx] = real_disk.dev_path > if uri: > result["DISK_%d_URI" % idx] = uri > if disk.name: > diff --git a/lib/storage/extstorage.py b/lib/storage/extstorage.py > index 10d766b..311662f 100644 > --- a/lib/storage/extstorage.py > +++ b/lib/storage/extstorage.py > @@ -142,6 +142,16 @@ class ExtStorageDevice(base.BlockDev): > self.dev_path = result[0] > self.uris = result[1:] > > + if not self.dev_path: > + logging.info("A local block device is not available") > + self.dev_path = None > + if not self.uris: > + logging.error("Neither a block device nor a userspace URI is > available") > + return False > + > + self.attached = True > + return True > + > # Verify that dev_path exists and is a block device > try: > st = os.stat(self.dev_path) > diff --git a/man/ganeti-extstorage-interface.rst > b/man/ganeti-extstorage-interface.rst > index 75ca639..49631b0 100644 > --- a/man/ganeti-extstorage-interface.rst > +++ b/man/ganeti-extstorage-interface.rst > @@ -129,6 +129,23 @@ The attach script should be idempotent if the volume > is already mapped. > If the requested volume is already mapped, then the script should just > return to its stdout the path which is already mapped to. > > +In case the storage technology supports userspace access to volumes as > +well, e.g. the QEMU Hypervisor can see an RBD volume using its embedded > +driver for the RBD protocol, then the provider can return extra lines > +denoting the available userspace access URIs per hypervisor. The URI > +should be in the following format: <hypervisor>:<uri>. For example, a > +RADOS provider should return kvm:rbd:<pool>/<volume name> in the second > +line of stdout after the local block device path (e.g. /dev/rbd1). > + > +So, if the ``access`` disk parameter is ``userspace`` for the ext disk > +template, then the QEMU command will end up having file=<URI> in > +the ``-drive`` option. > + > +In case the storage technology supports *only* userspace access to > +volumes, then the first line of stdout should be an empty line, denoting > +that a local block device is not available. If neither a block device > +nor a URI is returned, then Ganeti will complain. > + > detach > ~~~~~~ > > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs > index a071a8c..37b645e 100644 > --- a/src/Ganeti/OpCodes.hs > +++ b/src/Ganeti/OpCodes.hs > @@ -631,7 +631,7 @@ $(genOpCode "OpCode" > ], > "instance_name") > , ("OpInstanceActivateDisks", > - [t| [(NonEmptyString, NonEmptyString, NonEmptyString)] |], > + [t| [(NonEmptyString, NonEmptyString, Maybe NonEmptyString)] |], > OpDoc.opInstanceActivateDisks, > [ pInstanceName > , pInstanceUuid > -- > 1.7.10.4 > > Hrvoje Ribicic Ganeti Engineering Google Germany GmbH Dienerstr. 12, 80331, München Geschäftsführer: Graham Law, Christine Elizabeth Flores Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
