CP-5817: Productise removal of lvm2-environment-device.patch #28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I reviewed the patch and believed the patch has correctly implemented the resolution suggested in CP-5817. My only concern is the potential performance regression, as the previous performance issue for which the original patches were introduced was reported by more than one big customers in their production environments. But if we've decided this is no longer a issue or a compromise we'd like to undertake, I've nothing to against the implementation itself here. |
|
Also please note that, whoever to do the final integration should make sure to integrate the other pull request on guest-packages.hg first and then this one second (or at the same time). |
|
Thanks for the review Zheng. We discussed the consequences of the rollback with Keith and he things we should be ok. CP-5817 talks about an alternative implementation which will be our choice if we see any performance regression. |
Reviewed-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com> GitHub: closes xapi-project#34 on xapi-project/sm
It seems that the xenbuilder chroot env doesn't have /dev/tty. So in the build log, we saw "tee: /dev/tty: No such device or address" This is strangely different from the local chroot testing env I created using standard xenbuilder command "make sm-chroot" when doing test. This quick fix avoids using the "tee /dev/tty" trick and do it in a plain/verbose way. Signed-off-by: Zheng Li <zheng.li@eu.citrix.com> Reviewed-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com>
Don't use the primary VHD footer for file-based VHD VDIs that are attached in R/W mode because it has been erased. Use the back-up one instead. Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com> Reviewed-by: Keith Petley <keith.petley@citrix.com> GitHub: closes xapi-project#35 on xapi-project/sm
…ev-rules
This patch is the counterpart of 2 patches removed in
dm-multipath (formerly dm-multipath in guest-packages.hg):
- dm-multipath-disable-udev-rules
- dm-multipath-dm-event
This patch basically does the following:
- ship and override multipath udev rules from sm RPM
- add rules to invoke mpathcount from udev
Note: for udev to catch the relevant events, the kernel must have
DM_UEVENT enabled
Signed-off-by: Germano Percossi <germano.percossi@citrix.com>
Reviewed-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com>
GitHub: closes xapi-project#40 on xapi-project/sm
This changeset basically reinstate previous code, backing out "changeset: 1555:ac091775c8ea" in the Mercurial repository. To be safe, we remove the key before adding, just in case. Signed-off-by: Germano Percossi <germano.percossi@citrix.com> Reviewed-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com> GitHub: closes xapi-project#41 on xapi-project/sm
The rate XAPI is notified is throttled to avoid spamming (currently defaults to one message per hour, per SR). This rate can be overridden by setting the "coalesce-error-rate" key in an SR's other-config. Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com> Suggested-by: Andrei Lifchits <andrei.lifchits@citrix.com> Reviewed-by: Germano Percossi <germano.percossi@citrix.com> GitHub: closes xapi-project#38 on xapi-project/sm
Don't bother updating the vhd-blocks in the XAPI database as this is owned by the GC and there's no synchronisation for this. Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com> Reviewed-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com> GitHub: closes xapi-project#37 on xapi-project/sm
This functionality has stopped being a patch to upstream multipath and it is shipped in its own startup script provided by sm RPM Signed-off-by: Germano Percossi <germano.percossi@citrix.com> Reviewed-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com> GitHub: closes xapi-project#42 on xapi-project/sm
My previous pylint change 96e56bf replaced self.ty as it was not defined anywhere in the VDI object. But this breaks some horrible hack in the underlying layer, which was probably implementated that way for backward "compatability" reason by itself. Signed-off-by: Zheng Li <zheng.li@eu.citrix.com> Reviewed-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com>
Another incompatability issue introduced by my pylint serie. On the other hand, the root cause was some bad code manifested due by this change. In some subclass of VDI.VDI, we set some fields before initializting the VDI.VDI parent class, so that these fields might be overwritten during the parent class initialization. As a common rule of thumb, we should generally do parent class initialization first and then do overwritten and extention in the following steps in the children initializor. And any information essential for parent class initialization should be defined as a parameter of the parent __init__ method. E.g. uuid is a predefined parameter of the initialization method in the parent class (VDI.VDI), but many children classes still don't use this parameter, and instead do uuid assignment in the children classes instead and then call the parent initializor. This patchset doesn't correct these issues for the moment, but we should pay attention to this, otherwise we might get bitten in the future. Signed-off-by: Zheng Li <zheng.li@eu.citrix.com> Reviewed-by: Vineeth Thampi Raveendran <vineeth.thampi@citrix.com> GitHub: closes xapi-project#43 on xapi-project/sm
Part of the change for rolling back lvm2-environment-device.patch. Removed code that exports special environment variable to direct LVM to look for an abbreviated device cache. The second part of the change (actual removal of lvm patch) is under guest-packages. Signed-off-by: Chandrikas Srinivasan <chandrika.srinivasan@citrix.com> Reviewed-by: Zheng Li <zheng.li@eu.citrix.com>
|
Reopening pull request. The original changeset hasn't changed but was rolled back and is being checked-in again. No review required. |
andrey-podko
referenced
this pull request
in andrey-podko/sm
Aug 16, 2022
Part of the change for rolling back lvm2-environment-device.patch. Removed code that exports special environment variable to direct LVM to look for an abbreviated device cache. The second part of the change (actual removal of lvm patch) is under guest-packages. Reviewed-by: Zheng Li <zheng.li@eu.citrix.com> Github: close xcp-ng#28 on xapi-project/sm
andrey-podko
referenced
this pull request
in andrey-podko/sm
Aug 16, 2022
Part of the change for rolling back lvm2-environment-device.patch. Removed code that exports special environment variable to direct LVM to look for an abbreviated device cache. The second part of the change (actual removal of lvm patch) is under guest-packages. Reviewed-by: Zheng Li <zheng.li@eu.citrix.com> GitHub: closes xcp-ng#28 on xapi-project/sm
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of the change for rolling back lvm2-environment-device.patch. Removed code
that exports special environment variable to direct LVM to look for an
abbreviated device cache.
The second part of the change (actual removal of lvm patch) is under
guest-packages.