After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 725384 - Wizard offers to change the disk size up to exabytes
Wizard offers to change the disk size up to exabytes
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: wizard
3.11.x
Other Linux
: Normal normal
: 3.22
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-02-28 09:45 UTC by Christophe Fergeau
Modified: 2016-03-31 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libvirt-machine-props: Fix max_storage calculation (1.75 KB, patch)
2014-03-08 22:55 UTC, Fabiano Fidêncio
none Details | Review
libvirt-machine-props: Volume size is correct units (1.48 KB, patch)
2014-03-12 23:05 UTC, Zeeshan Ali
committed Details | Review

Description Christophe Fergeau 2014-02-28 09:45:54 UTC
1) Start the wizard to create a new VM
2) Pick a live image (I used a 64bit f19 livecd)
3) Click on the 'customize' button
-> the disk size can't be changed (reported in another bug)
4) Go back
5) Cancel VM creation
6) Start the wizard to create a new VM
7) Pick the same image (or a different one, does not seem to matter)
8) Click on the 'customize' button
9) Disk size can be customized up to 18EB (exabytes)
Comment 1 Fabiano Fidêncio 2014-03-08 22:54:14 UTC
I sort of can reproduce this one.
Please, take a look in the attached patch.
Comment 2 Fabiano Fidêncio 2014-03-08 22:55:54 UTC
Created attachment 271339 [details] [review]
libvirt-machine-props: Fix max_storage calculation

As pool_info.available is not recalculated when the user changes the disk
size, leaves and re-enters in the Customize window, the max_storage value
must be calculated taking consideration the first min_storage value and
not the current disk size.
Comment 3 Zeeshan Ali 2014-03-09 21:40:20 UTC
Review of attachment 271339 [details] [review]:

why not re-fetch pool_info (its not calculated) instead each time it changes?
Comment 4 Fabiano Fidêncio 2014-03-09 22:16:12 UTC
(In reply to comment #3)

> why not re-fetch pool_info (its not calculated) instead each time it changes?

Please, define re-fetch.

According to my understanding of the code, it is exactly what the code was trying to do, no? I've tried to force a refresh () in the StoragePool, but it doesn't help either.
I'm not familiar with libvirt{,-gobject}, but looks like the StoragePool belongs to the StorageVol, which one is not updating the StoragePoolInfo when the resize is done. But I'm not even sure if it should update or not.

Christophe, I'd like to hear your opinion to know if it is a bug in libvirt-gobject.
Comment 5 Fabiano Fidêncio 2014-03-09 22:20:50 UTC
Oops, I meant, the StorageVol belongs to the StoragePool.
Comment 6 Zeeshan Ali 2014-03-09 22:57:41 UTC
(In reply to comment #4)
> (In reply to comment #3)
> 
> > why not re-fetch pool_info (its not calculated) instead each time it changes?
> 
> Please, define re-fetch.

As in update to the actual new value when it changes.
 
> According to my understanding of the code, it is exactly what the code was
> trying to do, no?

Not according to your commit log or from the code afaict.

> I've tried to force a refresh () in the StoragePool, but it
> doesn't help either.

Then the next step would be to investigate that.

> I'm not familiar with libvirt{,-gobject}, but looks like the StoragePool
> belongs to the StorageVol, which one is not updating the StoragePoolInfo when
> the resize is done. But I'm not even sure if it should update or not.

If you aim to continue working on Boxes, I'd advice you to get some basic familiarity with libvirt. Investigating the issue further would likely be a good step in that direction. :)
Comment 7 Fabiano Fidêncio 2014-03-09 23:04:42 UTC
(In reply to comment #6)
 
> Not according to your commit log or from the code afaict.

The current Boxes code, I meant.

> 
> If you aim to continue working on Boxes, I'd advice you to get some basic
> familiarity with libvirt. Investigating the issue further would likely be a
> good step in that direction. :)

Yeah, it's part of my weekend's TODO list.
Just need enough time to do that :-)
Comment 8 Zeeshan Ali 2014-03-10 11:22:07 UTC
(In reply to comment #7)
> (In reply to comment #6)
> 
> > Not according to your commit log or from the code afaict.
> 
> The current Boxes code, I meant.

Yes, that is what i meant too with 'the code'.
 
> > If you aim to continue working on Boxes, I'd advice you to get some basic
> > familiarity with libvirt. Investigating the issue further would likely be a
> > good step in that direction. :)
> 
> Yeah, it's part of my weekend's TODO list.
> Just need enough time to do that :-)

Cool, then I await the actual fix or a compelling rationale for the workaround.
Comment 9 Zeeshan Ali 2014-03-10 12:36:54 UTC
<teuf> fidencio: and you're getting the exabyte thing ? or just the max size grows little by littel ?
<fidencio> teuf: max size grows little by little
<teuf> fidencio: yup, probably different thing than what I was seeing

And its not reproducible in git master with the steps in comment#0.
Comment 10 Zeeshan Ali 2014-03-12 23:05:18 UTC
Created attachment 271665 [details] [review]
libvirt-machine-props: Volume size is correct units

GVir.DomainDisk.resize expects values in KiB.

I'm not entirely sure if it helps with the following bug but my guess is
that it does as the disk capacity really must have been set to that
insane value because of the issue this bug fixes.
Comment 11 Zeeshan Ali 2014-03-12 23:06:31 UTC
Comment on attachment 271665 [details] [review]
libvirt-machine-props: Volume size is correct units

Attachment 271665 [details] pushed as 5b51ef6 - libvirt-machine-props: Volume size is correct units