feature request - prevent user stupidity (eg mine)

feature request - prevent user stupidity (eg mine)

[James Harper]

I foolishly made the following typo:

"
disk = [ 'phy:/dev/vg00/virt-smtp1-root,0301,w',
'phy:/dev/vg00/virt-smtp1-root,0302,w' ]
"

instead of:

"
disk = [ 'phy:/dev/vg00/virt-smtp1-root,0301,w',
'phy:/dev/vg00/virt-smtp1-swap,0302,w' ]"

(the type was that the same Dom0 block device is used twice in the first
example)

Which was working great until I did a 'mkswap /dev/hda2' inside the
domain, which really fscked things up :(

Given that there is almost no way you would ever want to publish the
same block device to the same virtual domain more than once, would there
be any value in flagging that as an error when you try and start the
domain?

Thanks

James

RE: feature request - prevent user stupidity (eg mine)

[Ian Pratt]

> (the type was that the same Dom0 block device is used twice in the
first
> example)
> 
> Which was working great until I did a 'mkswap /dev/hda2' inside the
> domain, which really fscked things up :(
> 
> Given that there is almost no way you would ever want to publish the
> same block device to the same virtual domain more than once, would
there
> be any value in flagging that as an error when you try and start the
> domain?

I'm surprised that the existing block device safety checks don't pick up
on this, but I guess they're run before the guest is started rather than
before each virtual disk is attached.

I certainly think there's a strong argument for protecting the user from
this kind of error, at least unless they've specified 'w!' and hence
indicated they want sharing.

Anyone care to knock up a patch?

Best,
Ian

RE: feature request - prevent user stupidity (eg mine)

[Masaki Kanno]

>> (the type was that the same Dom0 block device is used twice in the
>first
>> example)
>> 
>> Which was working great until I did a 'mkswap /dev/hda2' inside the
>> domain, which really fscked things up :(
>> 
>> Given that there is almost no way you would ever want to publish the
>> same block device to the same virtual domain more than once, would
>there
>> be any value in flagging that as an error when you try and start the
>> domain?
>
>I'm surprised that the existing block device safety checks don't pick up
>on this, but I guess they're run before the guest is started rather than
>before each virtual disk is attached.
>
>I certainly think there's a strong argument for protecting the user from
>this kind of error, at least unless they've specified 'w!' and hence
>indicated they want sharing.
>
>Anyone care to knock up a patch?

Hi Ian,

I already posted a patch. Could you comment about the patch?

http://lists.xensource.com/archives/html/xen-devel/2007-06/msg00856.html

Best regards,
 Kan

Re: feature request - prevent user stupidity (eg mine)

[Mark Williamson]

> > Given that there is almost no way you would ever want to publish the
> > same block device to the same virtual domain more than once, would
>
> there
>
> > be any value in flagging that as an error when you try and start the
> > domain?
>
> I'm surprised that the existing block device safety checks don't pick up
> on this, but I guess they're run before the guest is started rather than
> before each virtual disk is attached.

Looks like the hotplug scripts do contain the checking logic, and it does 
check each device at a time, but it explicitly ignores if the same VM already 
has access to a device.  Maybe checking for duplicate exports here would be 
too racey, since I guess the hotplug scripts are not run in any particular 
order...

> I certainly think there's a strong argument for protecting the user from
> this kind of error, at least unless they've specified 'w!' and hence
> indicated they want sharing.

Agreed.  It's something that - whilst fairly unlikely - is easy to do in the 
right circumstances.  And those circumstances usually lead to hosing off your 
filesystem quite effectively.

I looked at Kan's patch, which implements this checking within XendConfig.py.  
It's not an awful lot of extra code, however it does seem like this stuff 
would ideally be done in one place - either in Xend or in the hotplug 
scripts.

The nice thing about the hotplug scripts is that they Just Work for 
dynamically adding devices.  Unless I've misread Kan's code, it won't catch a 
user accidentally adding a duplicate blkdev to a running domain, which is a 
shame.

We should try to figure out if this can be added reliably to the blkdev 
hotplug scripts.  If not, maybe we should put all the checking into Xend, 
with hooks to validate added devices.

Cheers,
Mark

-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

Re: feature request - prevent user stupidity (eg mine)

[Daniel P. Berrange]

On Fri, Jun 29, 2007 at 01:05:18AM +0100, Mark Williamson wrote:
> > > Given that there is almost no way you would ever want to publish the
> > > same block device to the same virtual domain more than once, would
> >
> > there
> >
> > > be any value in flagging that as an error when you try and start the
> > > domain?
> >
> > I'm surprised that the existing block device safety checks don't pick up
> > on this, but I guess they're run before the guest is started rather than
> > before each virtual disk is attached.
> 
> Looks like the hotplug scripts do contain the checking logic, and it does 
> check each device at a time, but it explicitly ignores if the same VM already 
> has access to a device.  Maybe checking for duplicate exports here would be 
> too racey, since I guess the hotplug scripts are not run in any particular 
> order...

Well duplicate devices in the same VM could be fairly easily checked by XenD
itself before it gets anywhere near creating the VM or doing device hotplug.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

Re: feature request - prevent user stupidity (eg mine)

[Mark Williamson]

> > Looks like the hotplug scripts do contain the checking logic, and it does
> > check each device at a time, but it explicitly ignores if the same VM
> > already has access to a device.  Maybe checking for duplicate exports
> > here would be too racey, since I guess the hotplug scripts are not run in
> > any particular order...
>
> Well duplicate devices in the same VM could be fairly easily checked by
> XenD itself before it gets anywhere near creating the VM or doing device
> hotplug.

Kan's patch does this for initial domain creation and could easily be extend 
to cover adding devices (thinking about it, it may do already but I'd have to 
check the Xend internets), and is fine as far as it goes.

It just doesn't seem clean to have the device safety logic in Xend *and* in 
the hotplug script.

Cheers,
Mark


-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

Re: feature request - prevent user stupidity (eg mine)

[Masaki Kanno]

>> > Given that there is almost no way you would ever want to publish the
>> > same block device to the same virtual domain more than once, would
>>
>> there
>>
>> > be any value in flagging that as an error when you try and start the
>> > domain?
>>
>> I'm surprised that the existing block device safety checks don't pick up
>> on this, but I guess they're run before the guest is started rather than
>> before each virtual disk is attached.
>
>Looks like the hotplug scripts do contain the checking logic, and it does 
>check each device at a time, but it explicitly ignores if the same VM 
>already 
>has access to a device.  Maybe checking for duplicate exports here would be 
>too racey, since I guess the hotplug scripts are not run in any particular 
>order...
>
>> I certainly think there's a strong argument for protecting the user from
>> this kind of error, at least unless they've specified 'w!' and hence
>> indicated they want sharing.
>
>Agreed.  It's something that - whilst fairly unlikely - is easy to do in the 
>right circumstances.  And those circumstances usually lead to hosing off your 
>filesystem quite effectively.
>
>I looked at Kan's patch, which implements this checking within XendConfig.py.  
>It's not an awful lot of extra code, however it does seem like this stuff 
>would ideally be done in one place - either in Xend or in the hotplug 
>scripts.
>
>The nice thing about the hotplug scripts is that they Just Work for 
>dynamically adding devices.  Unless I've misread Kan's code, it won't catch a 
>user accidentally adding a duplicate blkdev to a running domain, which is a 
>shame.

Could you teach me a blkdev definition or a adding blkdev operation that 
my code cannot catch that you think of?  I'd like to test them by my code. 


>We should try to figure out if this can be added reliably to the blkdev 
>hotplug scripts.  If not, maybe we should put all the checking into Xend, 
>with hooks to validate added devices.

I'd like to check a duplicate blkdev for both xm new command and xm create 
command, so that I added the checking into xend(XendConfig.py).  
If the duplicate blkdev is notified of by xm start command, I think that 
it is late.

Best regards,
 Kan

Re: feature request - prevent user stupidity (eg mine)

[Mark Williamson]

> >The nice thing about the hotplug scripts is that they Just Work for
> >dynamically adding devices.  Unless I've misread Kan's code, it won't
> > catch a user accidentally adding a duplicate blkdev to a running domain,
> > which is a shame.
>
> Could you teach me a blkdev definition or a adding blkdev operation that
> my code cannot catch that you think of?  I'd like to test them by my code.

I was thinking your code didn't get run at block-attach, but later on I 
started to think maybe it was :-)  Is it? ;-)

> >We should try to figure out if this can be added reliably to the blkdev
> >hotplug scripts.  If not, maybe we should put all the checking into Xend,
> >with hooks to validate added devices.
>
> I'd like to check a duplicate blkdev for both xm new command and xm create
> command, so that I added the checking into xend(XendConfig.py).
> If the duplicate blkdev is notified of by xm start command, I think that
> it is late.

Ah, I hadn't thought about xm new.  Well, in that case it sounds fairly 
reasonable to add to Xend, and your code looked OK to me.  Given that, I'd 
have no objections to your patch going in.  Might save me from PBKAC (Problem 
Between Keyboard And Chair) one day :-)

Cheers,
Mark

-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

Re: feature request - prevent user stupidity (eg mine)

[Masaki Kanno]

>> >The nice thing about the hotplug scripts is that they Just Work for
>> >dynamically adding devices.  Unless I've misread Kan's code, it won't
>> > catch a user accidentally adding a duplicate blkdev to a running domain,
>> > which is a shame.
>>
>> Could you teach me a blkdev definition or a adding blkdev operation that
>> my code cannot catch that you think of?  I'd like to test them by my code.
>
>I was thinking your code didn't get run at block-attach, but later on I 
>started to think maybe it was :-)  Is it? ;-)

I tested xm block-attach command by my patch.  I believe it's okay. 

# xm list --long vm1
(domain
<<snip>>
    (device
        (vbd
            (uname file:/xen/rhel4u2.root.img-vm1)
            (uuid 40221979-3528-afe0-a018-da8e4b956590)
            (mode w)
            (dev hda1:disk)
            (backend 0)
            (bootable 1)
        )
    )
<<snip>>
# xm block-attach vm1 file:/xen/rhel4u2.root.img-vm1 hdb1 w
Error: The uname "file:/xen/rhel4u2.root.img-vm1" is already defined
Usage: xm block-attach <Domain> <BackDev> <FrontDev> <Mode> [BackDomain]

Create a new virtual block device.

Best regards, 
 Kan


>> >We should try to figure out if this can be added reliably to the blkdev
>> >hotplug scripts.  If not, maybe we should put all the checking into Xend,
>> >with hooks to validate added devices.
>>
>> I'd like to check a duplicate blkdev for both xm new command and xm create
>> command, so that I added the checking into xend(XendConfig.py).
>> If the duplicate blkdev is notified of by xm start command, I think that
>> it is late.
>
>Ah, I hadn't thought about xm new.  Well, in that case it sounds fairly 
>reasonable to add to Xend, and your code looked OK to me.  Given that, I'd 
>have no objections to your patch going in.  Might save me from PBKAC (
>Problem 
>Between Keyboard And Chair) one day :-)
>
>Cheers,
>Mark
>
>-- 
>Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
>Mark: To answer a question with a question: What use is a skateboard?
>Dave: Skateboards have wheels.
>Mark: My wheel has a wheel!
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel