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

Send blockdev change signal on grow physical #3372

Merged
merged 3 commits into from
Jul 11, 2023

Conversation

mulkieran
Copy link
Member

@mulkieran mulkieran commented Jul 6, 2023

Closes #3371

@mulkieran mulkieran self-assigned this Jul 6, 2023
@mulkieran mulkieran requested a review from bgurney-rh July 6, 2023 19:34
@jbaublitz jbaublitz self-requested a review July 6, 2023 20:17
Comment on lines 874 to 851
prop_hashmap!(
consts::BLOCKDEV_INTERFACE_NAME_3_3 => {
Vec::new(),
consts::BLOCKDEV_TOTAL_SIZE_PROP.to_string() =>
box_variant!(total_physical_size_prop.clone())
},
consts::BLOCKDEV_INTERFACE_NAME_3_4 => {
Vec::new(),
consts::BLOCKDEV_TOTAL_SIZE_PROP.to_string() =>
box_variant!(total_physical_size_prop.clone())
},
consts::BLOCKDEV_INTERFACE_NAME_3_5 => {
Vec::new(),
consts::BLOCKDEV_TOTAL_SIZE_PROP.to_string() =>
box_variant!(total_physical_size_prop.clone())
},
consts::BLOCKDEV_INTERFACE_NAME_3_6 => {
Vec::new(),
consts::BLOCKDEV_TOTAL_SIZE_PROP.to_string() =>
box_variant!(total_physical_size_prop)
}
),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the best thing to do here. Even if grow physical was added in r3, the total physical size property is available all the way back so theoretically even if a user is using a r3 method, we should notify them that the property in r1 (for example) has changed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

@mulkieran
Copy link
Member Author

mulkieran commented Jul 7, 2023

I think that the best way to get the object path corresponding to the device is to relinquish the Some-Write lock and get an AllRead lock. I was overthinking this a bit, I think.

@mulkieran mulkieran force-pushed the issue_stratisd_3371 branch 4 times, most recently from 44d6b4a to 5e6457b Compare July 7, 2023 18:16
@mulkieran mulkieran marked this pull request as ready for review July 7, 2023 19:08
@mulkieran mulkieran requested a review from jbaublitz July 7, 2023 19:09
@bgurney-rh
Copy link
Member

The new stratisd_cert test that was added today has a --monitor-dbus failure for this DifferentProperty message:

DifferentProperty(dbus.ObjectPath('/org/storage/stratis3/13'), dbus.String('org.storage.stratis3.blockdev.r0'), dbus.String('UserInfo'), dbus.Struct((dbus.Boolean(False), dbus.String('')), signature=None, variant_level=1), dbus.Struct((dbus.Boolean(True), dbus.String('eeeeeeeeee')), signature=None, variant_level=1))

@mulkieran
Copy link
Member Author

rebased

@mulkieran
Copy link
Member Author

mulkieran commented Jul 8, 2023

The new stratisd_cert test that was added today has a --monitor-dbus failure for this DifferentProperty message:

DifferentProperty(dbus.ObjectPath('/org/storage/stratis3/13'), dbus.String('org.storage.stratis3.blockdev.r0'), dbus.String('UserInfo'), dbus.Struct((dbus.Boolean(False), dbus.String('')), signature=None, variant_level=1), dbus.Struct((dbus.Boolean(True), dbus.String('eeeeeeeeee')), signature=None, variant_level=1))

CI should be passing again w/ #3375

@mulkieran
Copy link
Member Author

rebased.

@mulkieran
Copy link
Member Author

@mvollmer Please let us know if you want to test this.

@mulkieran
Copy link
Member Author

rebased + squashed

@mulkieran mulkieran requested review from bgurney-rh and jbaublitz and removed request for mvollmer July 10, 2023 23:28
@mulkieran
Copy link
Member Author

Now all new and revised!

@mulkieran
Copy link
Member Author

Squashing a bit before merging...

From now on, if the pool size changes, then so will some other
properties, so we will likely always need to go with the pool foreground
change methods.

Signed-off-by: mulhern <[email protected]>
@mulkieran mulkieran removed the request for review from bgurney-rh July 11, 2023 15:51
@mulkieran mulkieran merged commit 84e5bda into stratis-storage:master Jul 11, 2023
33 checks passed
@mulkieran mulkieran deleted the issue_stratisd_3371 branch July 11, 2023 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pool.GrowPhysicalDevice doesn't trigger change notification for blockdev.TotalPhysicalSize
3 participants