Re: [PATCH] Add glusterfs volumes support to virt-install

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/02/2014 04:35 AM, Anatoly Belikov wrote:
---
  tests/xmlparse-xml/change-disk-in.xml  |  8 +++++++
  tests/xmlparse-xml/change-disk-out.xml |  8 +++++++
  tests/xmlparse.py                      |  9 ++++++++
  virtinst/cli.py                        | 28 ++++++++++++++++++++----
  virtinst/devicedisk.py                 | 23 +++++++++++---------
  virtinst/diskbackend.py                | 39 +++++++++++++++++++++++++++-------
  virtinst/storage.py                    | 14 +++++++++---
  7 files changed, 104 insertions(+), 25 deletions(-)


Thanks for updating the patch. A few comments:

- There's still spurious whitespace changes (added/removing blank lines, changing indentation of unaltered code)

- Supporting a single disk hostname isn't sufficient, at least ceph/rbd can handle multiple host names

- The libxml usage means we are doing something wrong: we should be using StoragePool's parsing capabilities.

- The change should be split further: one patch handling the source protocol, source name, and host name additions, the another patch handling the gluster:// URI parsing magic

- tests/clitest.py should be extended as well

That said, I just finished setting up a gluster and ceph server for testing, so I'm working on this now. I'll likely massage this patch and commit the protocol/name/hostname bit with proper attribution, and build on it from there. I'll respond here when there's more to test

Thanks,
Cole

_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list




[Index of Archives]     [Linux Virtualization]     [KVM Development]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux