Thanks, Glenn. I'll get that CR filed to track the TODO.
-Drew
On 5/17/11 10:42 AM, Glenn Lagasse wrote:
* Drew Fisher ([email protected]) wrote:
Matt and Glenn,
Drew,
Comment in logical.py :
"(although we can destroy the zvol)"
This is not true, because we cannot unassign the zvol as a dump
device, the zvol itself cannot be destroyed, and therefore the pool
cannot be destroyed either, so I reckon best to remove this portion
of the comment.
Fixed.
Glenn, we discussed this in detail with Drew, and CUD AI will trap
on this scenario if a manifest is specifying to remove a dump zvol
device, and put appropriate information to the log stating how to
work around this. Essentially we are not allowing it, until 6910925
is addressed.
I guess having CUD AI trap for this is 'ok' but it seems less than
ideal to me to support something that can't actually be fully
accomplished. Requiring all consumers of logical.py to know that dump
zvols have to be treated specially until some dumpadm deficiency is
addressed could lead to issues imho if that subtlety isn't
caught/understood. The code as-is doesn't actually work if the zvol in
question is a dump zvol and some higher level consumer doesn't filter it
out. So in essence, we aren't really addressing anything to do with
dump zvols over what the code already did/tried to do. It would seem to
make more sense to me to actually include code to detect if a dump zvol
is what we're trying to destroy and actually not allow it since that's
the reality until the dumpadm issue is addressed.
I've added a check to the code that scans dumpadm output for the
zvol. If the zvol is detected in the output, we don't try to
destroy the zvol at all. I also log to the debug log a message to
ensure that anybody using logical.py knows about the deficiency.
There's also the issue of how does this code gets 'fixed' when/if the
dumpadm issue gets resolved (we're closing our bug that purportedly
allows for the deletion of the dump zvol, but not really since we're not
removing the zvol from dumpadm first).
I guess at a minimum I'd expect a new bug to be filed against logical.py
for the TODO to finish the support for removing a dump zvol and link the
dumpadm bug with it so that we'll at least have some way to track this.
Although I think just not allowing dump zvols to be destroyed since they
can't be is really the right thing to do imo.
I will file a bug to track the TODO and link it to both 6910925 and
7006603 (AI shouldn't create dump device prior to installation)
Could I get you guys to take a look at the new webrev?
http://cr.opensolaris.org/~drewfish/cr_7044383/
Looks good to me. Thanks for addressing this Drew.
Cheers,
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
