Disk add dialog: Change format together with the storage pool #1331
Conversation
|
Some flakes, not sure if relevant. |
|
A better step would obviously be to drop the effect entirely. I'll see if I can find some time to experiment with that. In the meantime, @KKoukiou do you think this is an improvement, or does it make the thing even more complicated? |
If you want to take it a step further, check the useEffect for the 'diskParams.file' state. It also has logic for changing the device and the format. |
martinpitt
force-pushed
the
format-race
branch
from
818a8cd to
dfededb
Compare
Thanks for pointing out! It's not quite the same, as that's for a pool vs. a file, but it would make sense to factorize the logic. I'll think about that. |
|
@KKoukiou I didn't see any significant overlap in the end. But I did clean up the effect a bit to avoid an unnecessary and racy state update. |
martinpitt
force-pushed
the
format-race
branch
from
c60fcb6 to
81e95b9
Compare
|
Meh, every time I take a stab at this code, it crumbles into pieces. I'll back out 81e95b9 , let's at least fix the first issue. |
martinpitt
force-pushed
the
format-race
branch
from
81e95b9 to
3a08bc7
Compare
Wait until the auto-detection of the disk format for an iso file is finished before setting it. Otherwise it could happen that the auto-detection changes the value after the test.
Otherwise we create a race condition: The ModalBody already gets re-rendered with the new pool name, but with possibly with a format that is invalid for the new pool type. That triggers our new assertion. This is very redundant with the effect which fine-tunes the format and disk type according to the other data. This effect is evil and should be dropped, but doing so requires more intrusive code cleanup. Introduce a helper function to avoid repeated code.
martinpitt
force-pushed
the
format-race
branch
from
3a08bc7 to
eaf797d
Compare
Otherwise we create a race condition: The ModalBody already gets re-rendered with the new pool name, but with possibly with a format that is invalid for the new pool type. That triggers our new assertion.
This is very redundant with the effect which fine-tunes the format and disk type according to the other data. This effect is evil and should be dropped, but doing so requires more intrusive code cleanup. Introduce a helper function to avoid repeated code.
Follow-up from #1327 (comment)