Xen devel - Jun 2007 - feature request - prevent user stupidity (eg mine)

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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> (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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>> (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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> > 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!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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  -=| 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> > 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!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>> > 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> >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!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>> >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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel