CP-6697: Add "coalesce" flag to SR other-config

When coalesce=false is set, SR GC will not do any coalesce (including
leaf coalesce) on the given SR. The rational is as follows:

Some times 3rd party user (e.g. CloudStack) need to introduce an
existing storage share (e.g. NFS) as a XenServer SR. These shares
might contain some VDI files. This is all normal. However, these VDI
files might be VHD chains (unlike the VHD "trees" in XenServer
VDI as no snapshot stub node exists). So when the SR is introduced,
XenServer will coalesce the chain automatically. The senario of
VHD chains are considered valid, but the consequence is not what
users normally want.

We already had a other-config:gc flag with which the user can
disable the GC completely. But unfortunately, the basic VDI.destroy
function also depends on GC. When we delete a file or LVM based VHD,
the VHD file is not immediately deleted but only marked as hidden.
It's up to the GC to actually remove the files and do all the
cleanup steps following that. With GC completely disabled, the VHD
file will never be deleted on disk, so the space will never be
freed up.

So we introduced this coalesce flag, if set to false (and gc remains
the default true), only coalescing (including leaf-coalescing) will
be disabled and the normal prune (i.e. hidden -> delete) can still
take place.

Unfortunately, we later on found out that this is not directly
useful for the current installation of CloudStack, because when
CloudStack copies VDI snapshots to the secondary storage, it
does a literal copy using "dd" without doing re-parenting. So
that regardless what this patch does

  * GC always fails scanning their existing secondary storage
  * And if the scan had passed (or if we now let it skip the
error), since the VHD chain/tree can not be established without
correct parenting information and these VHDs all have their "hidden"
flag set to true, these VHD files would have been considered as
orphanced VHD roots which are ready for deletion.

This feature will become useful when CloudStack starts to use
the VDI.copy's new rebase mirror feature and the old installation
being converted or phased out.

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
CP-6697: Add "nosubdir" flag for NFSSR

The normal NFSSR creates a sub directory under the NFS directory
given by params "server:serverpath", and use that sub directory
as the SR's working directory.

This patch set add an extra flag "sm-config:nosubdir", which
defaults to false. But when "nosubdir=true", NFSSR will avoid
creating that sub directory. Instead the SR will mount and use
the "serverpath" directory as the working directly.

The purpose of this patch is to provide a behavior compatible
with CloudStack's current NFSSR fork (which always use the
"nosubdir" semantics), so that they can move to the official
NFSSR with the help of this flag.

The reasons for choosing sm-config instead of other key/value
maps are:

  * device-config is RO map, so one can not change it's value
without migrating all VMs away in order to unplug the PBD
and recreate one . This will add considerable burden to CS's
already quite complicated upgrade process from the NFSSR fork
to the new one.

  * other-config is RW map, but it's not a param for sr-create.
So setting nosubdir during sr-create is no-op. One must then
reset the flag and unplug/plug the PBD to make it effective.

This patch also takes care of some corner case when someone
creates both a standard NFSSR and a "nosubdir" NFSSR under the
same serverpath. Ideally, they shouldn't interfere with each
other. This could be useful if CloudStack wants to gradually
moave all their current nosubdir NFSSRs to standard NFSSRs for
their primary storage.

The patch is partially based on a previous version implemented by
Anthony Xu <Xuefei.Xu@citrix.com>.

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
CP-6697: Fix FileSR capability config and param docs

This patch fix the outdated and incorrect configuration of
FileSR capabilities config and param doc. They are registered
by SM with XAPI, which will ultimately reflects on things like
XE tab completion and XE helps etc.

As we can see from the existing FileSR.py implementation, FileSR's
mandatory device-config param is not "path" but "location".

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
CP-6697: Refactor _checkmount to base class FileSR

It's common for all its sub classes, and now we intend to use it
in the base class (FileSR) as well, so we do the refactoring by
moving it to the base class instead.

Note that util.ismount doesn't seem to be very different from the
standard os.path.ismount. It might date back to days when the
os.path.ismount doesn't exist. Here we intentionally keep it
unchanged for backward compatability, in case the code needs to
be backport to earlier releases (though quite unlikely).

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
CP-6697: Modernize FileSR as an independent SR type

Currently, almost all the file-based SR types have two paths in
concept and implementation:

  * remotepath: the source path of the storage, e.g. a NFS share's
    address like server:/serverpath

  * path: the local path in /var/run/sr-mount/$sr_uuid, where
    the remotepath is mounted to

However, FileSR, the base class of all file-based SR types, doesn't
follow this convention itself. It doesn't have the  "remotepath"
varible; instead it defines the source path of the storage to be
the "path" (which should, conceptually, be the "remotepath"); it
doesn't have an entry in /var/run/sr-mount like other SRs.

This leads to two disadvantages:

  * When FileSR is used as an independent SR types, many of the
    the library code and utilities doesn't work with it, because
    they assume the existance of both variables or the entry in
    /var/run/sr-mount. E.g., SM GC assumes that all the SRs it
    works with must have a /var/run/sr-mount entry. Issues like
    this haven't been spot before, because FileSR hasn't been
    used as an indepedent SR type for a long time, also some of
    the methods that could potentially tigger these issues were
    shadowed by method overloading in the children classes.

  * This inconsistency also reduces the chance of code sharing.
    Some functions that could have been implemented in the base
    class can't be defined in the base class, because e.g. there
    is no "remotepath" variable defined or the "path" variable
    has a meaning conceptually different from the one in the
    child class. These code would then have to be duplicated into
    each children class and involve on their own, which is
    quite error prone.

A big part of this patch is to address this issue by creating
the same conceptual model for FileSR.

  * The source path of the storage which passed in as
    device-config:location (like before) is defined as the
    "remotepath"

  * There is now a /var/run/sr-mount entry, where the
    "remotepath" is bind mount onto, which is the "path". This
    is conceptually consistent with all other file-based SR types,
    and implementation-wise too.

With these changes, most of the library code will immediately
work with FileSR with no changes required on them (which is our
point). And FileSR then becomes an independent SR type out of box.

A little bit of change is done to attach (and similarly detach):
we now avoid calling super().attach from children classes, because
the attach method in the base class now does more work specific
to FileSR itself.

We haven't done much refactoring as part of this patch itself,
though it should introduce some potentials for code refactoring
and reusing in the future.

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
CP-6697: normal ismount can not deal with bind mount

This is a known design issue of bind mount on Linux. As we
now use bind mount for FileSR, we need to fix this. Here we
implement a customized version specific to our environment.

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Merge CP-6697

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>

GitHub: closes #58 on xapi-project/sm
CA-122950: Ignore invalid VDI handle exceptions in SR.scan

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Reviewed-by: Zheng Li <zheng.log@citrix.com>

GitHub: closes #61 on xapi-project/sm
CA-122253: Improve error reporting for LVM-based SR snapshots

Return the actual error instead of returning errors that might have
occurred in the error-handling path.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Reviewed-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com>

GitHub: closes #60 on xapi-project/sm
CA-125137: sr_ref doesn't always exist

E.g. when doing SR.probe, sr_ref doesn't exist.

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
CA-125122: Ensure the existance of remotepath when calling _isbind

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Merge CP-6697 fixup

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>

GitHub: closes #63 on xapi-project/sm
revert merge 2ef774c98e12aa072d3cd0d0ec90971521aaef93

This commit reverts merge 2ef774c98e12aa072d3cd0d0ec90971521aaef93,
which contains fixes for regressions introduced by merge
0f0a4809d8df4ec31ff1d2096b2a2e5fc9784c4c (CP-6697). We are revert this
merge because we don't have time to test it. The commits reverted are:
* 6cf9efdef7984aec28e51f5a45f2ad3373fa3a8b: "CA-125122: Ensure the existance
  of remotepath when calling _isbind"
* f03332c3c82f1b9d893cf3328d6a166febd6e502: "CA-125137: sr_ref doesn't always
  exist"

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Reviewed-by: Germano Percossi <germano.percossi@citrix.com>
revert merge 0f0a4809d8df4ec31ff1d2096b2a2e5fc9784c4c

This commit reverts merge 0f0a4809d8df4ec31ff1d2096b2a2e5fc9784c4c
(CP-6697) because that merge introduced significant regressions. The
commits that are reverted are:
 * eed0ec8f4b2fc01e97e49a1bd5913501b7b53c39: "CP-6697: normal ismount can not
   deal with bind mount"
 * 34b9bb382e06ad952fb491446b47d65e6f64860d: "CP-6697: Modernize FileSR as an
   independent SR type"
 * 691cd9782b28eccd5d6662b34ea8fae42c8cb6cb: "CP-6697: Refactor _checkmount to
   base class FileSR"
 * d80fe84f04be06db746ad0694ef90b4b0f1f86b5: "CP-6697: Fix FileSR capability
   config and param docs"
 * 8eb8126f0e02284776b0989dd5c4e51eb446f941: "CP-6697: Add "nosubdir" flag for
   NFSSR"
 * 3437a417b03ac6456ee9deab732c7b37e3980aa1: "CP-6697: Add "coalesce" flag to
   SR other-config"

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Reviewed-by: Germano Percossi <germano.percossi@citrix.com>
Merge pull request #64 from tmakatos/master

revert commits that broke the storage BST and the nightly
revert 666f0b56 (CA-122253)

This commit reverts "CA-122253: Improve error reporting for LVM-based SR
snapshots" as it hasn't been tested by a storage BST and the imminent
backout merge from trunk-storage into trunk could introduce regressions.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Reviewed-by: Germano Percossi <germano.percossi@citrix.com>

GitHub: closes #65 on xapi-project/github
CP-6697: Add "coalesce" flag to SR other-config

When coalesce=false is set, SR GC will not do any coalesce (including
leaf coalesce) on the given SR. The rational is as follows:

Some times 3rd party user (e.g. CloudStack) need to introduce an
existing storage share (e.g. NFS) as a XenServer SR. These shares
might contain some VDI files. This is all normal. However, these VDI
files might be VHD chains (unlike the VHD "trees" in XenServer
VDI as no snapshot stub node exists). So when the SR is introduced,
XenServer will coalesce the chain automatically. The senario of
VHD chains are considered valid, but the consequence is not what
users normally want.

We already had a other-config:gc flag with which the user can
disable the GC completely. But unfortunately, the basic VDI.destroy
function also depends on GC. When we delete a file or LVM based VHD,
the VHD file is not immediately deleted but only marked as hidden.
It's up to the GC to actually remove the files and do all the
cleanup steps following that. With GC completely disabled, the VHD
file will never be deleted on disk, so the space will never be
freed up.

So we introduced this coalesce flag, if set to false (and gc remains
the default true), only coalescing (including leaf-coalescing) will
be disabled and the normal prune (i.e. hidden -> delete) can still
take place.

Unfortunately, we later on found out that this is not directly
useful for the current installation of CloudStack, because when
CloudStack copies VDI snapshots to the secondary storage, it
does a literal copy using "dd" without doing re-parenting. So
that regardless what this patch does

  * GC always fails scanning their existing secondary storage
  * And if the scan had passed (or if we now let it skip the
error), since the VHD chain/tree can not be established without
correct parenting information and these VHDs all have their "hidden"
flag set to true, these VHD files would have been considered as
orphanced VHD roots which are ready for deletion.

This feature will become useful when CloudStack starts to use
the VDI.copy's new rebase mirror feature and the old installation
being converted or phased out.

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
CP-6697: Add "nosubdir" flag for NFSSR

The normal NFSSR creates a sub directory under the NFS directory
given by params "server:serverpath", and use that sub directory
as the SR's working directory.

This patch set add an extra flag "sm-config:nosubdir", which
defaults to false. But when "nosubdir=true", NFSSR will avoid
creating that sub directory. Instead the SR will mount and use
the "serverpath" directory as the working directly.

The purpose of this patch is to provide a behavior compatible
with CloudStack's current NFSSR fork (which always use the
"nosubdir" semantics), so that they can move to the official
NFSSR with the help of this flag.

The reasons for choosing sm-config instead of other key/value
maps are:

  * device-config is RO map, so one can not change it's value
without migrating all VMs away in order to unplug the PBD
and recreate one . This will add considerable burden to CS's
already quite complicated upgrade process from the NFSSR fork
to the new one.

  * other-config is RW map, but it's not a param for sr-create.
So setting nosubdir during sr-create is no-op. One must then
reset the flag and unplug/plug the PBD to make it effective.

This patch also takes care of some corner case when someone
creates both a standard NFSSR and a "nosubdir" NFSSR under the
same serverpath. Ideally, they shouldn't interfere with each
other. This could be useful if CloudStack wants to gradually
moave all their current nosubdir NFSSRs to standard NFSSRs for
their primary storage.

The patch is partially based on a previous version implemented by
Anthony Xu <Xuefei.Xu@citrix.com>.

PS. the fix for an issue (CA-125137) caused by a previous version
of this patch is now integrated in this version.

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
CP-6697: Fix FileSR capability config and param docs

This patch fix the outdated and incorrect configuration of
FileSR capabilities config and param doc. They are registered
by SM with XAPI, which will ultimately reflects on things like
XE tab completion and XE helps etc.

As we can see from the existing FileSR.py implementation, FileSR's
mandatory device-config param is not "path" but "location".

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
CP-6697: Refactor _checkmount to base class FileSR

It's common for all its sub classes, and now we intend to use it
in the base class (FileSR) as well, so we do the refactoring by
moving it to the base class instead.

Note that util.ismount doesn't seem to be very different from the
standard os.path.ismount. It might date back to days when the
os.path.ismount doesn't exist. Here we intentionally keep it
unchanged for backward compatability, in case the code needs to
be backport to earlier releases (though quite unlikely).

Signed-off-by: Zheng Li <zheng.li@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>