Adjusting how DASDs are handled as recently ID_* tags are also included
in the udev information which causes the problems reported by
https://issues.redhat.com/browse/RHEL-39497
Removing the filtering of offline ccw devices as these devices are
available in the system and also are used to set them online again. The
state information is made available in the ccw capability as an optional
state element.

Boris Fiuczynski (4):
  nodedev: refactor storage type fixup
  nodedev: improve DASD detection
  nodedev: prevent invalid DASD node object creation
  nodedev: add ccw device state and remove fencing

 src/conf/node_device_conf.c        | 24 +++++++++++++
 src/conf/node_device_conf.h        | 11 ++++++
 src/conf/schemas/nodedev.rng       |  8 +++++
 src/node_device/node_device_udev.c | 56 +++++++++++++++++++++++++-----
 4 files changed, 90 insertions(+), 9 deletions(-)

-- 
2.45.0

On 6/19/24 14:29, Boris Fiuczynski wrote:
> Adjusting how DASDs are handled as recently ID_* tags are also included
> in the udev information which causes the problems reported by
> https://issues.redhat.com/browse/RHEL-39497
> Removing the filtering of offline ccw devices as these devices are
> available in the system and also are used to set them online again. The
> state information is made available in the ccw capability as an optional
> state element.
> 
> Boris Fiuczynski (4):
>   nodedev: refactor storage type fixup
>   nodedev: improve DASD detection
>   nodedev: prevent invalid DASD node object creation
>   nodedev: add ccw device state and remove fencing
> 
>  src/conf/node_device_conf.c        | 24 +++++++++++++
>  src/conf/node_device_conf.h        | 11 ++++++
>  src/conf/schemas/nodedev.rng       |  8 +++++
>  src/node_device/node_device_udev.c | 56 +++++++++++++++++++++++++-----
>  4 files changed, 90 insertions(+), 9 deletions(-)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and merged.

Michal

On 6/20/24 9:41 AM, Michal Prívozník wrote:
> On 6/19/24 14:29, Boris Fiuczynski wrote:
>> Adjusting how DASDs are handled as recently ID_* tags are also included
>> in the udev information which causes the problems reported by
>> https://issues.redhat.com/browse/RHEL-39497
>> Removing the filtering of offline ccw devices as these devices are
>> available in the system and also are used to set them online again. The
>> state information is made available in the ccw capability as an optional
>> state element.
>>
>> Boris Fiuczynski (4):
>>    nodedev: refactor storage type fixup
>>    nodedev: improve DASD detection
>>    nodedev: prevent invalid DASD node object creation
>>    nodedev: add ccw device state and remove fencing
>>
>>   src/conf/node_device_conf.c        | 24 +++++++++++++
>>   src/conf/node_device_conf.h        | 11 ++++++
>>   src/conf/schemas/nodedev.rng       |  8 +++++
>>   src/node_device/node_device_udev.c | 56 +++++++++++++++++++++++++-----
>>   4 files changed, 90 insertions(+), 9 deletions(-)
>>
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 
> and merged.
> 
> Michal

Thanks, Michal.

-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Thank you, Boris.

Is the following test scenario sufficient to describe this change, are you aware of any other scenarios that might be at risk of seeing a regression, maybe some scenario for other storage devices like SCSI?

Given a DASD disk with identifier 0.0.4024 on subchannel 0.0.0030
When I deconfigure the device (chzdev -d 0.0.4024)
And I configure the device (chzdev -e 0.0.4204)
And I request the xml details of node devices (virsh nodedev-list --storage)
Then the DASD disk is listed
And its parent is a device with capability ccw and identifier 0.0.4024
And the parent of that device is another device with capability css and identifier 0.0.0030

On 6/19/24 5:10 PM, smitterl@redhat.com wrote:
> Thank you, Boris.
> 
> Is the following test scenario sufficient to describe this change, are you aware of any other scenarios that might be at risk of seeing a regression, maybe some scenario for other storage devices like SCSI?
> 
> Given a DASD disk with identifier 0.0.4024 on subchannel 0.0.0030
> When I deconfigure the device (chzdev -d 0.0.4024)
> And I configure the device (chzdev -e 0.0.4204)
> And I request the xml details of node devices (virsh nodedev-list --storage)
I am not aware of a "storage" option on command nodedev-list. My guess 
is that your are referring to "nodedev-list --tree".

> Then the DASD disk is listed
> And its parent is a device with capability ccw and identifier 0.0.4024
> And the parent of that device is another device with capability css and identifier 0.0.0030

The first three patches do not actually change existing behavior but 
instead fix the problem described in the referenced issue.
When the ccw device is configured (online) the relationships are and 
also should have been as you describe above.

Theses patches are not modifying SCSI storage device node behavior.

-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

> On 6/19/24 5:10 PM, smitterl(a)redhat.com wrote:
> I am
> not aware of a "storage" option on command nodedev-list. My guess 
> is that your are referring to "nodedev-list --tree".
I meant the '--cap storage' option. But I see the '--tree' also works and it additionally displays the hierarchy that my test usually would traverse by checking dumpxml and there the parents.
> 
> The first three patches do not actually change existing behavior but 
> instead fix the problem described in the referenced issue.
> When the ccw device is configured (online) the relationships are and 
> also should have been as you describe above.
> 
> Theses patches are not modifying SCSI storage device node behavior.
Understood.

I was able to confirm that with the scratch build Jiŕi Denemark provided
a) the device is listed only once
b) the device' parent is correctly set (ccw instead of css)
c) no critical product regression hit in our test suite tp-libvirt for test set 'virsh.nodedev_list,virsh.nodedev_dumpxml'