Bug 1470007 - [RFE] [libvirt part] Add S3 PR support to qemu (similar to mpathpersist)
[RFE] [libvirt part] Add S3 PR support to qemu (similar to mpathpersist)
Status: ON_QA
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: libvirt (Show other bugs)
7.4
Unspecified Linux
urgent Severity high
: rc
: 7.5
Assigned To: Michal Privoznik
yisun
: FutureFeature, Upstream
Depends On: 1519019 1533158 1464908 1484075
Blocks: RHEV_SCSI_reserve_Win_DirectLUN RHEV_SCSI_reserve_Win_SharedDisk 1477664 1519021 1558125 1457437
  Show dependency treegraph
 
Reported: 2017-07-12 05:13 EDT by Martin Tessun
Modified: 2018-06-13 03:53 EDT (History)
19 users (show)

See Also:
Fixed In Version: libvirt-4.4.0-1.el7
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: 1464908
: 1519021 1533158 (view as bug list)
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Comment 1 Paolo Bonzini 2017-08-22 12:29:55 EDT
Upstream libvirt discussion:
https://www.redhat.com/archives/libvir-list/2017-August/msg00631.html
Comment 7 Michal Privoznik 2018-01-18 11:06:47 EST
Patches for the basic operations were posted here:

https://www.redhat.com/archives/libvir-list/2018-January/msg00584.html

What's missing is the counterpart for qemu emitting event on pr-helper disappearing. That'll be implemented once qemu implements it.
Comment 8 Michal Privoznik 2018-02-21 13:13:15 EST
Another try:

https://www.redhat.com/archives/libvir-list/2018-February/msg01021.html
Comment 11 Michal Privoznik 2018-05-16 04:13:58 EDT
I've pushed patches upstream:

b0cd8045f0 qemu: Detect pr-manager-helper capability
eba6467fed qemu_hotplug: Hotunplug of reservations
3f968fda7b qemu_hotplug: Hotplug of reservations
053d9e30e7 qemu: Start PR daemon on domain startup
8be74af168 qemu: Introduce pr_helper to qemu.conf
d13179fe8d qemu_cgroup: Allow /dev/mapper/control for PR
5bf89434ff qemu_ns: Allow /dev/mapper/control for PR
13fe558fb4 qemu: Generate pr cmd line at startup
3c28602759 qemu: Introduce pr-manager-helper capability
c7c9dea0a0 qemuDomainDiskChangeSupported: Deny changing reservations
687730540e virstoragefile: Introduce virStoragePRDef

Also, couple of fixes by Peter that are needed:

9b3cbd33a7 qemu: hotplug: Replace qemuDomainDiskNeedRemovePR
8bebb2b735 util: storage: Store PR manager alias in the definition
26c72a76dc conf: domain: Add helper to check whether a domain def requires use of PR
b4f113ee44 qemu: command: Move check whether PR manager object props need to be built
8f7c25ae39 qemu: process: Change semantics of functions starting PR daemon
b571e7bad0 qemu: Assign managed PR path when preparing storage source
e31f490458 util: storage: Allow passing <source> also for managed PR case
900fc66121 util: storage: Drop virStoragePRDefIsEnabled
e72b3f0bbe util: storage: Drop pointless 'enabled' form PR definition
1efda36765 qemu: Move validation of PR manager support
64e3ae0d51 qemu: command: Fix comment for qemuBuildPRManagerInfoProps
b5aec60cc4 qemu: alias: Allow passing alias of parent when generating PR manager alias
90309bcdc5 qemu: hotplug: Fix spacing around addition operator


v4.3.0-217-g9b3cbd33a7
Comment 14 yisun 2018-06-06 00:07:21 EDT
Hi Michal, 
I saw there is a new commit about this function recently as follow:
======
commit 105bcdde76bc8c64f2d9aca9db684186a5e96e63
Author: Peter Krempa <pkrempa@redhat.com>
Date:   Thu May 31 15:18:20 2018 +0200

    qemu: hotplug: Fix detach of disk with managed persistent reservations
    
    In commit 8bebb2b735d I've refactored how the detach of disk with a
    managed persistent reservations object is handled. After the commit if
    any disk with a managed PR object would be removed libvirt would also
    attempt to remove the shared 'pr-manager-helper' object potentially used
    by other disks.
    
    Thankfully this should not have practical impact as qemu should reject
    deletion of the object if it was still used and the rest of the code is
    correct
=====

Do we need to let this bz depend on that commit, too?
Comment 15 Michal Privoznik 2018-06-06 06:54:35 EDT
(In reply to yisun from comment #14)
> 
> Do we need to let this bz depend on that commit, too?

Yes, we should backport that commit. I'll do that in a while.

Note You need to log in before you can comment on or make changes to this bug.