[virt-tools-list] [PATCH 0/5] Make filesystem devices editable

[Cole Robinson]

On 01/21/2014 04:05 AM, Cédric Bosdonnat wrote:
> This patch serie initial goal was to make filesystem devices editable, but I also
> discovered that there were some filesystem types and drivers missing for the LXC
> cases. The code and UI from the addhardware.py was extracted into a new vmmFSDetails
> class and UI file to help reusing it in the details.py code.
> 
> Cédric Bosdonnat (5):
>   Add Hardware: added the missing filesystem types for LXC guests.
>   Add lxc filesystem drivers: loop and nbd
>   Create a separate class vmmFSDetails to share filesystem addhardware UI
>   Use vmmFSDetails in details dialog to allow editing filesystem devices
>   Write back the changes in filesystem details page
> 
>  tests/xmlparse-xml/change-filesystems-in.xml  |  13 +
>  tests/xmlparse-xml/change-filesystems-out.xml |  15 +
>  tests/xmlparse.py                             |  21 ++
>  ui/addhardware.ui                             | 331 +------------------
>  ui/details.ui                                 | 214 +-----------
>  ui/fsdetails.ui                               | 456 ++++++++++++++++++++++++++
>  virtManager/addhardware.py                    | 187 +----------
>  virtManager/details.py                        |  52 +--
>  virtManager/domain.py                         |  13 +
>  virtManager/fsdetails.py                      | 420 ++++++++++++++++++++++++
>  virtinst/devicefilesystem.py                  |  17 +-
>  11 files changed, 1001 insertions(+), 738 deletions(-)
>  create mode 100644 ui/fsdetails.ui
>  create mode 100644 virtManager/fsdetails.py
> 

Nice series! I've applied it now. I squashed patch 4 and 5 together since 4
was a bit misleading without 5.

I pushed these on good faith, with the expectation that you will provide a
follow up patches to address a few issues :)

- Having explicit UI for 'units' is overkill IMO, I'd rather just always
mandate the user specifies in MB like we do for the pre-existing memory UI.
Even if the user has GB as the unit in the XML, we can either just overwrite
it to MB, or convert it to GB when setting the memory value. I'll leave that
to your discretion

- Please move the unit conversion functions to virtinst/util.py, since that
units= XML pattern is used in several other places in libvirt XML, so we will
eventually want to share it.

- The NBD format list is basically duplicating the identical list already
available in virtinst/storage.py. Please find a way to share them. And in the
UI, just keep all the values lowercase, it looks even more awkward when they
are capitalized since most are acronyms (Iso, Qcow2, etc.)

- Using tests/testdriver.xml test-many-devices guest (see HACKING for details
on how to use this), just selecting an existing FS in the details window
throws an error for me:

Traceback (most recent call last):
  File "/home/crobinso/src/virt-manager/virtManager/details.py", line 1223, in
hw_selected
    self.refresh_filesystem_page()
  File "/home/crobinso/src/virt-manager/virtManager/details.py", line 3355, in
refresh_filesystem_page
    self.fsDetails.set_dev(dev)
  File "/home/crobinso/src/virt-manager/virtManager/fsdetails.py", line 212,
in set_dev
    self.set_config_value("fs-driver", dev.driver or "default")
  File "/home/crobinso/src/virt-manager/virtManager/fsdetails.py", line 244,
in set_config_value
    model_list = [x[0] for x in combo.get_model()]
TypeError: 'NoneType' object is not iterable

Please address that.

Thanks,
Cole