Bug 492082 - python-virtinst: add a label to newly created disks?
Summary: python-virtinst: add a label to newly created disks?
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: python-virtinst
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Cole Robinson
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 532746 581747 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-03-25 10:25 UTC by Mark McLoughlin
Modified: 2012-12-16 22:25 UTC (History)
23 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-12-16 22:06:59 UTC
Type: ---


Attachments (Terms of Use)
screenshot (70.58 KB, image/png)
2009-03-25 10:26 UTC, Mark McLoughlin
no flags Details
virtinst-0.400.3-empty-partition-table.patch (1.03 KB, patch)
2009-04-06 16:30 UTC, Mark McLoughlin
no flags Details | Diff

Description Mark McLoughlin 2009-03-25 10:25:12 UTC
If you run e.g. virt-install or virt-manager to create a new virtual machine, a new blank disk image is created.

anaconda throws this warning when it sees it:

  Error processing drive vda.
  Maybe it needs to be
  reinitialized. YOU WILL LOSE ALL DATA
  ON THIS DRIVE!

     [Ignore Drive] [Re-initialize drive]

Is there any way we can have anaconda be a little less terrifying for this very common case?

Comment 1 Mark McLoughlin 2009-03-25 10:26:02 UTC
Created attachment 336610 [details]
screenshot

Comment 2 Chris Lumens 2009-03-25 14:04:42 UTC
I don't know that we can tell the difference between a new, blank disk image and an existing disk that we cannot read for some reason.  I think the failures from parted are going to be the same.  So, we probably can't change the warning for just one case.

Comment 3 Mark McLoughlin 2009-03-25 14:34:41 UTC
Why not e.g. just read the first sector and see if it's all zeros?

Comment 4 Joel Andres Granados 2009-03-27 10:15:42 UTC
I *strongly* suggest keeping the terrifying message!  We need to make the user understand that what we are about to do is *very* destructive.  If he has new disks he will just ignore it. :)

Random comments:
1. Checking if the first sector is all zeros is not an option.  I can create a disk that has a Physical Volume on it with the first 512 byts zeroed out.  it does not mean it is invalid.

2. I think this approach is the safest considering the near infinite states that disk metadata can have (lvm, partitioning types, dmraid, mdriad, stuff I don't know about....)

3. I accept that this is definitely something we need to get better at.  anaconda does not accept a disk without a label (partition table) for example (only in some cases actually).  Which is retarded, given the fact that you can have a PV without a label.  But as long as we don't have a mechanism to detect said configurations, I *really* feel safer having this message there.

4. Detecting disk metadata is not trivial and it can get very complex.  Just to throw an example: A disk can have an msdos partition table, gpt partition table and lvm metadata.  And we have different apps doing different things in different places (parted, pyparted, udev, blkid...) and they all have to see the same thing.  It has happened to me that blkid says one thing and udev says another (not very often but ....)

Comment 5 Mark McLoughlin 2009-03-27 10:43:01 UTC
(In reply to comment #4)
> If he has new disks he will just ignore it. :)

Take the case of people using virt-manager to create a new guest. It is *not* an ideal default user experience to hit them with a "YOU WILL LOSE ALL DATA" warning.

If anaconda will never be able to tell the difference between a blank disk and a disk error, should virt-manager/virtinst initialize the disk before the install? Just write out an empty partition table?

Comment 6 Hans de Goede 2009-03-27 11:28:30 UTC
(In reply to comment #5)
> If anaconda will never be able to tell the difference between a blank disk and
> a disk error, should virt-manager/virtinst initialize the disk before the
> install? Just write out an empty partition table?  

IMHO yes it should.

Comment 7 Chris Lumens 2009-03-27 18:19:00 UTC
Writing out an empty partition table should be fairly straightforward with the new pyparted, though I have not given it a try.

Comment 8 Mark McLoughlin 2009-04-06 16:30:25 UTC
Created attachment 338355 [details]
virtinst-0.400.3-empty-partition-table.patch

So, here's a somple patch for virtinst to create an empty msdos partition table on the disk images it creates. It works great ... no scarey warning.

There are two obvious problems:

  - An msdos disk isn't always appropriate - parted.archLabels will help
    us with this:

      $> python
      >>> import parted
      >>> parted.archLabels
      {'ppc': ['msdos', 'mac', 'amiga', 'gpt'], 
       's390': ['dasd', 'msdos'], 
       'x86_64': ['msdos', 'gpt'], 
       'sparc': ['sun'], 
       'i386': ['msdos', 'gpt'], 
       'ia64': ['msdos', 'gpt'], 
       'alpha': ['bsd', 'msdos']}

    but anaconda has other smarts to e.g. figure out when it should use GPT

  - this code path in virtinst isn't much used anymore - we actually need this
    in libvirt's volume creation code more than we need it in virtinst

Moving to libvirt

Comment 9 Daniel Berrangé 2009-04-06 16:55:51 UTC
This is really painful todo in libvirt, particularly if the disk image for the guest is using a non-raw format like QCow2 / VMDK / etc. It also means we have to try and second guess which partition table format will be most desirable for the guest OS.  

Is there some argument available that virt-install could pass to anaconda to tell it to just go ahead & create a partition table ?

We already pass a 'method=$URL' option in behind the scenes, so if we could also add  'make-part-table'  option that'd be ideal.

Comment 10 Mark McLoughlin 2009-04-16 16:46:36 UTC
So, in summary:

  1) anaconda can't treat blank disks differently

  2) virt tools can't create non-blank disks without a serious amount of hackery

As Jeremy points out, the only viable solution may be to re-phrase the dialog to be less terrifying. It used to be worded much differently.

An example of a user being seriously confused by this:

  http://www.redhat.com/archives/fedora-virt/2009-April/msg00108.html

Comment 11 Martin Ebourne 2009-04-20 12:01:28 UTC
I agree with comment #4, de-scarifying the warning is a really bad idea until you can be absolutely certain there's no sane data on the drive.

For an example of a scenario where the warning comes up but you never want it to reinitialise the drive see bug #496535.

Re: comment #9 and a make-part-table option, would need to be sure that specified the exact device that needed the part table, otherwise adding a real lvm pv block device to a virtual machine could result in it being destroyed silently.

Maybe virtinst/libvirt could write a magic block to the start of any disks it creates and anaconda could detect that.

Comment 12 Eduardo Habkost 2009-04-20 14:55:43 UTC
(In reply to comment #11)
> 
> Maybe virtinst/libvirt could write a magic block to the start of any disks it
> creates and anaconda could detect that.  

There is a magic block that can be written, already: an empty partition table. Anaconda doesn't give any warning if it is going to create new partitions on space that is marked as unused on the partition table, right?

This has the advantage of working for many other OSes and Linux distributions.

Comment 13 Joel Andres Granados 2009-04-22 09:22:25 UTC
I think I have a workable compromise for this issue.  Why don't we keep the scary message and also add something along the lines of "This process is needed for new disks as well and will simply add a empty partition table".

The whole message must be changed as the additional information has to be coherent with the information that is already there.  Additionally we will have to postpone this for f12 since we are passed the string freeze and this issues is not at all a priority.

Comment 14 Mark McLoughlin 2009-04-22 09:29:53 UTC
(In reply to comment #13)
> this issues is not at all a priority.  

Incorrect. This "YOU WILL LOSE ALL DATA" message is displayed to every user who installs F11 in a virtual machine. That is a terrible user experience, and fixing it should very much be a priority.

Comment 15 Mark McLoughlin 2009-05-22 12:06:11 UTC
Moving to F12

Comment 16 Andrew McNabb 2009-06-08 19:35:01 UTC
No one has mentioned this yet, but there's another problem with the scary message: it makes it impossible to install non-interactively.  I ran across the message when installing a virtual machine over PXE.  My kickstart script should have been able to install without any interaction, so I was disappointed to have  to interact with a dialog.

Comment 17 Mark McLoughlin 2009-06-09 07:23:19 UTC
(In reply to comment #16)
> My kickstart script should have been able to install without any interaction

Try "clearpart --initlabel"

Comment 18 Roman Kagan 2009-07-02 16:07:18 UTC
clearpart --initlabel is of no help.

Note that this is a major regression because essentially unattended installation of Fedora is no longer possible.

At the very least there should be a means to shut this warning up in kickstart files.  Better still, to keep back compat, make this the default behavior in kickstart installations, and only break into interacting with a user if a special directive is given in kickstart file.

I suggest to raise the severity of this bug, and to retarget it at an F11 respin so that people could have a working unattended installation of F11.

Comment 19 Andrew McNabb 2009-07-02 16:57:18 UTC
Doing "clearpart --initlabel" seemed to solve it for me.  Roman, maybe you could try double-checking that the option was really set.

Comment 20 Joel Andres Granados 2009-07-13 11:15:11 UTC
Roman:
  If that behavior persists, pls open another bug.  --initlabel should work for kickstart.

Comment 21 Nicholas 2009-07-22 01:34:27 UTC
I am running Fedora 11 (anaconda 11.4.1.63 (i think))

I am having this issue and --initlabel does not seem to fix it.

That would be perfect for me, but it seems to have no affect.

Ideas?

Comment 22 Nicholas 2009-07-22 01:37:35 UTC
Actually it seems to be Anaconda 11.5.0.59

Comment 23 Joel Andres Granados 2009-09-17 10:07:50 UTC
jlaska:
Think we can close this?

Comment 24 Mark McLoughlin 2009-09-21 13:21:42 UTC
(In reply to comment #23)

> Think we can close this?  

Has it been fixed? What is the current warning message?

Looking at the code, I see it would be:

    Error processing drive:
    ...
    This device may need to be reinitialized.
    REINITIALIZING WILL CAUSE ALL DATA TO BE LOST!

That's still an incredibly scary message to print every time someone installs a new Fedora virtual machine

Comment 25 Chris Lumens 2009-10-28 14:39:16 UTC
Andrew, others - if you are seeing kickstart installs blocked by this dialog even when using clearpart/zerombr/whatever, please file a separate bug to track that.  I'd hate for this bug report to become even more cluttered.

Everyone else - I still maintain that this message is appropriate for anaconda given that what we are asking of users.  At the point this dialog appears, anaconda is preparing to completely wipe the disk of whatever's on it and create a new disk label.  If the user was not paying attention when they configured the system, or disks were detected in a different order than expected, or they're on some strange multiboot configuration, or a host of other issues, the user could be preparing to wipe a disk with useful data on it.  They will not be able to recover.  We really need to let the user know the gravity of this situation.  I guarantee that if we tone this message down, we will get bugs about people nuking their data because they didn't understand what was about to happen.

It looks like the virt tools are providing us with a disk that looks the same as a real uninitialized disk.  We don't have a way of differentiating these two situations so we can't simply adjust the message for the virt case.  Please, if you want anaconda to not display this message, use some of our many existing tools to create a disk image with a disk label on it.

Comment 26 Chris Lalancette 2009-11-04 08:54:38 UTC
*** Bug 532746 has been marked as a duplicate of this bug. ***

Comment 27 Chris Lalancette 2009-11-04 08:55:57 UTC
OK, re-opening and assigning back to python-virtinst for the time being.

Chris Lalancette

Comment 28 Kashyap Chamarthy 2009-11-04 09:59:03 UTC
shouldn't using a  "--force"(which assumes user knows what he's doing) with virt-install prevent all interactive prompts?

(I tried with --force on F11 with, and still get the same warning) 

FWIW, if --force *does* go ahead with accepting any prompts it should be fine. Or it would be useful if there is an extra "option" just to go ahead and create partition table as mentioned in Comment #9 

my 0.02 cents

/kashyap

Comment 29 Chris Lalancette 2009-11-04 10:28:10 UTC
(In reply to comment #28)
> shouldn't using a  "--force"(which assumes user knows what he's doing) with
> virt-install prevent all interactive prompts?
> 
> (I tried with --force on F11 with, and still get the same warning) 
> 
> FWIW, if --force *does* go ahead with accepting any prompts it should be fine.
> Or it would be useful if there is an extra "option" just to go ahead and create
> partition table as mentioned in Comment #9 

No, you are talking about something completely different.  --force tells virt-install itself not to ask any questions.  The "Error processing drive sda Maybe it needs to be reinitialized. YOU WILL LOSE ALL DATA ON THIS DRIVE" message is from *inside* the guest; virt-install has no control over that.  If you want to skip that message inside the guest, you need to change your kickstart to add "zerombr yes" (I believe), which will then go by that message.

The rest of this BZ is about virt-install actually adding a label to the guest disk on a new disk creation, which is very closely related, but not exactly the same thing (although it will achieve the same result).

Chris Lalancette

Comment 30 Kashyap Chamarthy 2009-11-04 11:44:19 UTC
thanks Chris for clarifying.

and also thanks "zerombr yes" tip (using "clearpart --initlabel" also worked as mentioned by Mark. )

Comment 31 Mark McLoughlin 2009-11-19 10:16:17 UTC
Just so it doesn't get missed, see comment #8, comment #9 and attachment #338335 [details] for more background on the issues involved with getting virt tools to create a label on the disk before starting the guest

Comment 32 Cole Robinson 2010-04-13 16:59:48 UTC
*** Bug 581747 has been marked as a duplicate of this bug. ***

Comment 33 Alok Kataria 2010-07-22 20:47:24 UTC
Is this bug already fixed ? We are also seeing this problem with VMware VM's.

If not what is the plan of action are you modifying the virt tools to create a disk label before guest creation ?
Could there be a generic solution in anaconda of detecting that this is a new disk and silently going ahead and creating a label for it ?

Comment 34 Alok Kataria 2010-07-22 20:48:32 UTC
Forgot to mention the VMware related bug is PR 616668

Comment 35 Cole Robinson 2010-07-23 18:03:44 UTC
There really isn't a plan in the tools, because adding a label to disk image formats like qcow2 or vmdk is AIUI very programmatically difficult, and we would have to duplicate partitioning logic that currently lives in anaconda.

And since we will likely be using qcow2 as the default in Fedora in the midterm future, hacking in a fix for raw format will only temporarily help (and is still non-trivial, since it would really need to be exposed in the libvirt API and then wired up in the tools, along with the above partitioning logic issue).

Re-adding installer team, maybe someone has some new ideas.

Comment 36 Daniel Berrangé 2010-07-23 18:16:07 UTC
Aside from Coles' point about this being very difficult with non-raw disks, virt-install should not be in the business of deciding what the best partition table format is (msdos vs gpt vs etc). The guest OS should be deciding this based on what it prefers/supports.

Comment 37 Alok Kataria 2010-07-23 21:02:19 UTC
I fully agree with Daniel here.
Brings back to the question that I asked ealier, maybe you guys have already thought about this question below.

> Could there be a generic solution in anaconda of detecting that this is a new
> disk and silently going ahead and creating a label for it ?

Comment 38 Brian Lane 2010-07-26 22:19:00 UTC
I don't see this as a problem. The newly created image is blank, and we treat it the same way we treat every other blank disk. If the dialog is a surprise to the user the first time I expect that they will get used to it after the 2nd or 3rd time they see it.

Comment 39 Cole Robinson 2010-07-27 12:34:48 UTC
> I don't see this as a problem. The newly created image is blank, and we treat
> it the same way we treat every other blank disk. If the dialog is a surprise to
> the user the first time I expect that they will get used to it after the 2nd or
> 3rd time they see it.    

Unfortunately getting used to a warning like this is a recipe for disaster: if the user is trained to ignore it, they may eventually ruin actual data.

Comment 40 Bug Zapper 2010-07-30 10:37:49 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 14 development cycle.
Changing version to '14'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

Comment 41 Cole Robinson 2012-01-18 13:59:02 UTC
Still relevant, moving to rawhide.

I think libguestfs can get us around the programmatic difficulties here. Rich, we can use libguestfs for adding a dos/gpt disk label here, right?

Comment 42 Richard W.M. Jones 2012-01-18 14:45:59 UTC
Yes.  virt-format is probably relevant to this discussion:
https://www.redhat.com/archives/libguestfs/2012-January/msg00136.html

Comment 43 Cole Robinson 2012-12-16 22:06:59 UTC
Based on the myriad of issues here, and the fact that it has lingered for so long, I'm just closing this. We could use libguestfs to add a label, but as pointed out in comment #36 it's not our job to decide what the best label for the job is. And really there hasn't been much complaint about this issue.

Comment 44 Richard W.M. Jones 2012-12-16 22:25:28 UTC
Agreed .. it's not a job for libvirt/virt-install nor anaconda.

If the user is concerned, they can create a suitably formatted
disk using virt-format / guestfish, before running virt-install.

Or they can do a kickstart install with the clearpart command.


Note You need to log in before you can comment on or make changes to this bug.