> On Wed, Oct 17, 2007 at 01:49:05PM -0400, Cole Robinson wrote: > > S.Sakamoto wrote: > > > Hi > > > > > > I make a patch of BugzillaID-329091. > > > https://bugzilla.redhat.com/show_bug.cgi?id=329091 > > > > > > Thanks, > > > Shigeki Sakamoto. > > > > > > ---------------------------------------------------------------------- > > > diff -r 531b73491ac2 virt-install > > > --- a/virt-install Wed Oct 10 14:24:48 2007 -0400 > > > +++ b/virt-install Tue Oct 16 16:01:29 2007 +0900 > > > @@ -15,6 +15,7 @@ > > > > > > > > > import os, sys, string > > > +from stat import * > > > from optparse import OptionParser, OptionValueError > > > import subprocess > > > import logging > > > @@ -48,10 +49,10 @@ def get_disk(disk, size, sparse, guest, > > > if not size is None: > > > msg = _("Please enter the path to the file you would like to use for storage. It will have size %sGB.") %(size,) > > > disk = cli.prompt_for_input(msg, disk) > > > - if os.path.exists(disk) and os.path.isfile(disk): > > > + if os.path.exists(disk) and ( S_ISBLK(os.stat(disk)[ST_MODE]) or S_ISREG(os.stat(disk)[ST_MODE]) ): > > > while 1: > > > retryFlg = False > > > - warnmsg = _("You are going to overwrite file '%s'!\n") % disk > > > + warnmsg = _("You are going to overwrite '%s'!\n") % disk > > > res = cli.prompt_for_input(warnmsg + _("Do you really want to use the file (yes or no)? ")) > > > try: > > > if cli.yes_or_no(res) is True: > > > ---------------------------------------------------------------------- > > > > Hello, > > > > I don't think this is the right approach. From the bug report, I agree > > that we shouldn't make it so easy to ruin a block device without some > > kind of precaution, but the right way to do it would be to have something > > that would require the user to explicitly specify the type of storage > > desired. Such as --file be just for file based storage, and add a --blkdev > > or some such option to specify device based storage. Or maintain --file as > > the storage location and add a --disktype option. > > > > Unfortunately this would break the existing interface, but so would > > this patch, which would require interaction anytime someone wanted > > to specify a device for storage. > > Yep, this is not acceptable. If we force a prompt for all block devices, > then the user will just end up using the override flag all the time, > defeating the purpose of checking. Using a separate --blkdev flag does > not solve the problem either, since they could still easily overwrite > the host filesystem. > > > Anyone else have an opinion on the right direction to go? I think > > having an explicit way to specify storage would be handy anyways. > > Nope, this doesn't solve anything. The problem is that given a block > device path /dev/sdN, there is no easy way to determine if it is already > used in the host. You can check if its mounted, but that's useless of > the volume is used as Swap. It also misses if the device is in fact a > physical volume in LVM. It also misses if the device is setup via dmcrypt. > > It is frankly a loosing battle to try & guess if a device is in use in > the host. Xen does a minimal attempt by looking to see if it is mounted > but that misses the swap & LVM & device mapper scenarios. I don't see us > being able todo any better in virt-install. > > > The only viable long term approach is to develop the libvirt storage APIs, > allowing the admin to explicitly design certain devices as being available > for guest usage ahead of time. That would let virt-install validate paths > given to it for sanity > The check of disk is the same as a check of file, I thought that is good. That is why I have made it such a check. But, I understand that it was not a radical solution of the problem. Sorry, Alteration is taking time. Shigeki Sakamoto. _______________________________________________ et-mgmt-tools mailing list et-mgmt-tools@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/et-mgmt-tools
