Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CP-9983 #232

Merged
merged 69 commits into from
Dec 1, 2014
Merged

CP-9983 #232

merged 69 commits into from
Dec 1, 2014

Conversation

tmakatos
Copy link
Contributor

Merge all relevant commits from xs64bit into master.

Signed-off-by: Thanos Makatos thanos.makatos@citrix.com

siddharthv and others added 30 commits November 19, 2014 14:48
Signed-off-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>
Signed-off-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>
Signed-off-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit d60003a)
In preparation for read caching, change actimeo=0 to acdirmin=0,acdirmax=0.
Using "actimeo=0" prevents caching file attributes which means that each read
requires a GETATTR call to the NFS server.  This prevents read caching from
being effective.  Changing the mount options to "acdirmin=0,acdirmax=0"
prevents caching directory attributes but allows caching file attributes.  This
allows read caching to be effective.

This reverts the change made to the mount options in CA-63601.  The issue in
that ticket was a transient issue, fixed by a later build, before the change to
the mount options was made.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 8e505ad)
Pass the "-D" argument to tap-ctl when opening an EXTFileVDI or an NFSFileVDI
to allow read caching by removing O_DIRECT.

Allow disabling read caching per-SR by setting other-config:o_direct to true.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 4be47e0)

Conflicts:

	drivers/blktap2.py
With static VDIs, the SR code is run without a XAPI session.  In this
case, set O_DIRECT to be on for safety since there is external knob to
adjust it.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 6c2d966)
The xapi plugin 'trim' could be used to enable trim on LVM based
SRs to free up storage space from Storage arrays. The plugin creates
a LV using all of the free space in the VG, and then issue a lvremove
setting the issue_discards to 1. lvremove should trigger a trim / unmap
by lvm that should result in freeing up the storage space in the array

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

(cherry picked from commit a34ef9c)

Conflicts:

	drivers/lvutil.py
Signed-off-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 6e6afbc)
Limited SR trim capability to LVHD based SRs. Also error handling
was modified to return a XML string response on failure containing
errcode and errmsg as a key value pair. Trim functionality was
separated from the trim plugin entry point.

Signed-off-by: Vineeth Raveendran <vineeth.thampi@citrix.com>
Reviewed-by: Mate Lakat <mate.lakat@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 1ae07e0)

Conflicts:

	Makefile
Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Reviewed-by: Vineeth Raveendran <vineeth.thampi@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit b2db2a9)
- Removed all code references
- Removed all CSLG error codes
- Replaced CSLGXMLParse to XMLParse

Signed-off-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 0b1e794)

Methods util.get_isl_scsiids, util.get_scsiid_from_svid, and
util.getCslDevPath have been kept back as they are used by OCFS.

Conflicts:

	drivers/mpathcount.py
Signed-off-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Reviewed-by: Mate Lakat <mate.lakat@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 004465e)
Mimic the behavior of rmdir, and also raise the appropriate exceptions
if client wants to open a file in a non-existing directory. Enable the
client to open files with 'w' flag.

Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 3090c20)
Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 019f140)
Concurrent refcounter operations on two different refcounters sharing
the same namespace can lead to race conditions:
 A.) By the time the refcounter file is written, the namespace directory
     has already been removed
 B.) Removing a namespace directory fails, because a a new refcounter file
     has been created within the same directory

_writeCount now returns False if the file was not found

A is solved by spinning on _writeCount until it succeeds.
B is solved by ignoring the errors

Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@cirix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 63491d7)

Conflicts:

	drivers/refcounter.py
Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit f5693a1)
Coalesce is an asynchronous operation so if a tapdisk refresh fails the
VM will lose the disk but the administrator will realise that when the
VM hangs. This patch notifies the toolstack as soon as the failure
occurs.

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

(cherry picked from commit 5d3a649)
Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Reviewed-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 4b999e1)

Conflicts:

	drivers/XE_SR_ERRORCODES.xml
Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 2508b26)
Signed-off-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 51d1ff4)
By printing the log messages to stdout, they will be captured by nose,
thus not disturbing the user. Should a test fail, the messages will
still be displayed.

Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Acked-by: Germano Percossi <germano.percossi@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 9a19651)
Should the user want to use a custom context implementation, he can use
the with_custom_context decorator to do that. It enables users to
inherit from TestContext, extend its functionality, and use the extended
version in a test. This could be useful if someone wanted to use a
custom implementation for `open`.

Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Acked-by: Germano Percossi <germano.percossi@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 4228963)
This test will reproduce the issue that was seen in the referenced
ticket. Whenever a new Lock instance is created, and the file() call
fails, the instance's lockfile attribute remained uninitialised. As the
instance has a custom destructor, that will be called by the garbage
collector. That function assumes that the instance has a lockfile
attribute, and thus prints a message.

Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Acked-by: Germano Percossi <germano.percossi@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 9c077a9)
As Lock's destructor was assuming that lockfile attribute exists, it
printed out an AttributeError on stdout while the GC was cleaning it up.
This fix initialises the lockfile field in the constructor.

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

(cherry picked from commit 22d7e9b)
Extract the file call to a separate method, so that the extracted method
could be used in tests as a seam, emulating situations like: what would
happen if the file call failed.

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

(cherry picked from commit 1d2d908)
These are test cases covering the lock module, and also testing what
happens if file() raises an IOError. This can happen as a race
condition, if two locks - A and B are created in the same namespace and
by the time A gets to open the lockfile, B already cleaned up the
namespace.

Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Acked-by: Germano Percossi <germano.percossi@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit d10e3b0)
If creating the lockfile failed with IOError having ENOENT error code,
that means that another lock within the same namespace already cleaned
up the namespace. With this change, should IOError happen with ENOENT,
the lock will re-create the namespace, and the file 10 times, and only
bubble up the exception if it failed all those retries.

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

(cherry picked from commit 3021503)
Test mount calls.

Signed-off-by: Robert Breker <robert.breker@citrix.com>
Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Acked-by: Germano Percossi <germano.percossi@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 27fbd54)
This enables the specification of the NFS version (3,4) in the
device-config of ISOSRs(type=nfs) and NFSSRs.

For example:

    device-config:nfsversion=4

Signed-off-by: Robert Breker <robert.breker@citrix.com>
Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Reviewed-by: Germano Percossi <germano.percossi@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 7fa6b8c)
Mate Lakat and others added 10 commits November 21, 2014 10:46
Testing the behavior of lvutil.remove.

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

(cherry picked from commit b41370f)
Test functionality for lvcreate size_in_percentage, additional
config_param arg for lvremove

Signed-off-by: Vineeth Raveendran <vineeth.thampi@citrix.com>
Reviewed-by: Mate Lakat <mate.lakat@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit bf822ba)
More robust version. Supersedes commit 050d45d

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

(cherry picked from commit a121c30)
Supersedes commit 801f9e9

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

(cherry picked from commit 3060073)
Commit a121c30 was about creating a tap disk when passing a physical
cd device to a guest.
For that purpose, we checked that the value of sr.sm_config['type'] was
equal to 'cd'.

With CA-147816, we also want a tap disk in case of a USB device.
However, sr.sm_config['type'] is equal to 'block' in this case, which
may be too general; for instance, we do not want to create a tap disk
for a storage array.
Moreover, there is no easy way to know that we are dealing with a USB
device.

According to the XenServer 6.2 administration guide, a udev SR only
handles cd and usb devices.
This fix relies on this assumption, since a tap disk is now created if
the SR handles udev devices.

Signed-off-by: Flavien Quesnel <flavien.quesnel@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit ce2e498)
Update and refactor unit tests introduced by 3060073 to reflect the
changes from a121c30 to c0f26bf.

Signed-off-by: Flavien Quesnel <flavien.quesnel@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit c71c968)
Problem: If an exception other than SR.SRException was raised in
SRCommand.run(), it was only caught and logged into SMlog by LVHD.
All other SR drivers let it pass to xapi, which in turn truncated
the traceback / exception message, making it difficult to debug.

Fix: There is now a single, driver agnostic point inside
SRCommand.run() that logs all exceptions to SMlog. Traceback
is not truncated. NOTE: All individual lines are limited to 989
characters.

Signed-off-by: Kostas Ladopoulos <Konstantinos.Ladopoulos@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit ca832ba)
Signed-off-by: Kostas Ladopoulos <Konstantinos.Ladopoulos@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit fdac607)
Read caching is disabled if license does not allow it.

VDI.attach_from_config does not contain a XAPI session, because this is
called by HA before XAPI starts. We don't use read caching for static
VDIs because it does not make sense, we only occasionally write data to
such VDIs.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
[squashed commits]
Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
Reviewed-by: Felipe Franciosi <felipe.franciosi@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 494b2d7)

Conflicts:

	drivers/blktap2.py
This change makes sure that no calls to MPP RDAC drivers (which are not
supported anymore) are made through the storage layer. A check for RDAC LUN
always returns false but not before logging a warning if the RDAC LUN path
is found to be populated.

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

(cherry picked from commit 0f4c992)
@siddharthv
Copy link
Contributor

@tmakatos I've haven't looked into this completely but a couple of quick observations:

  • Unit tests using travis-ci seem to be a bit broke at the moment and needs fixing
  • Don't merge the NFSv4 changes into master yet

@tmakatos
Copy link
Contributor Author

I'll a look at travis-ci.

Why not merge the NFSv4 changes yet?

Mate Lakat and others added 3 commits November 25, 2014 12:36
Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
Imported-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 832e9ea)
Even though it is packaged in the main ISO and installed
by default, this patch will prevent the SR type to
be usable unless the proper script is run

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

(cherry picked from commit 5bfb0cb)
@matelakat
Copy link
Contributor

In util.py pylint reports an error:

E:284,24: Instance of 'int' has no 'EIO' member (no-member)

That's due to the fact that we are using errno as a variable. Let's fix this issue by not using errno for variables.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Suggested-by: Mate Lakat <mate.lakat@citrix.com>
@tmakatos
Copy link
Contributor Author

Thanks, @matelakat, I added a commit (https://github.com/tmakatos/sm/commit/80bb87bd56402e01f0d0a4f83c7aa80dc30b79c6) addressing the issue.

Thanos Makatos and others added 4 commits November 25, 2014 14:38
If a test fails only the first line of the test description is printed.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Serialisation of a class name has changed over the python versions, so
the test needed to be adjusted. Now only the error message is searched
in the log instead of expecting a full match.

Signed-off-by: Mate Lakat <mate.lakat@citrix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@citrix.com>
Suggested-by: Thanos Makatos <thanos.makatos@citrix.com>
Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
@coveralls
Copy link

Coverage Status

Coverage increased (+4.65%) when pulling cb39f5e on tmakatos:CP-9983-CP-5962 into f93497b on xapi-project:master.

Don't check if a XAPI session exists by comparing session with the empty
string because if a XAPI session does exist it returns a error string,
which evaluates to true.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>

(cherry picked from commit 3fd6375cc13cb4106153b15c31c62580fd7dcc89)
@coveralls
Copy link

Coverage Status

Coverage increased (+4.65%) when pulling b5c7890 on tmakatos:CP-9983-CP-5962 into f93497b on xapi-project:master.

@siddharthv
Copy link
Contributor

Looks good now 👍

@tmakatos tmakatos merged commit b5c7890 into xapi-project:master Dec 1, 2014
tmakatos pushed a commit that referenced this pull request Dec 1, 2014
Merge from Creedence.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Reviewed-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>

GitHub: closes #232 on xapi-project/sm
andrey-podko pushed a commit to andrey-podko/sm that referenced this pull request Aug 16, 2022
Merge from Creedence.

Reviewed-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>

GitHub: closes xapi-project#232 on xapi-project/sm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants