Bug 796526

At present, Eric is working on it:
https://www.redhat.com/archives/libvir-list/2012-February/msg00959.html

v2 posted to upstream
https://www.redhat.com/archives/libvir-list/2012-March/msg00188.html

In POST:
http://post-office.corp.redhat.com/archives/rhvirt-patches/2012-March/msg01041.html

Verified PASS with libvirt-0.9.10-5.el6.

# virsh dumpxml rhel6 |grep -i memory
  <memory unit='KiB'>1048576</memory>
  <currentMemory unit='KiB'>1048576</currentMemory>

(In reply to comment #6)
> Verified PASS with libvirt-0.9.10-5.el6.
> 
> # virsh dumpxml rhel6 |grep -i memory
>   <memory unit='KiB'>1048576</memory>
>   <currentMemory unit='KiB'>1048576</currentMemory>

It would also be worth testing the other improvements made in this patch, such as 'virsh setmem --size=900M' setting the size to 921600 KiB.

(In reply to comment #6)
> Verified PASS with libvirt-0.9.10-5.el6.
> 
> # virsh dumpxml rhel6 |grep -i memory
>   <memory unit='KiB'>1048576</memory>
>   <currentMemory unit='KiB'>1048576</currentMemory>

Yuandan, it seems patches of Comment 3 fix many places, including docs, and try different memory unit, so we probably also need to check them together, oh, Eric has commented this.

Regards,
Alex

Move back to ON_QA, and will continue to test the following items:
  blockResize: add flag for bytes
  docs: use correct terminology for 1024 bytes
  api: add overflow error
  util: new function for scaling numbers
  xml: share 'unit' in RNG
  xml: output memory unit for clarity
  storage: support more scaling suffixes
  xml: drop unenforced minimum memory limit from RNG
  xml: use long long internally, to centralize overflow checks
  xml: use better types for memory values
  xml: allow scaled memory on input
  virsh: add option aliases
  virsh: use option aliases
  virsh: add command aliases, and rename nodedev-detach
  virsh: improve storage unit parsing
  virsh: improve memory unit parsing

There are some issues in the following test:

15/16. error msg for --shrink option of vol-resize
# virsh vol-resize /var/lib/libvirt/images/rhel5.img 6000000000  --shrink
error: Failed to change size of volume 'rhel5.img' to 6000000000

error: invalid argument: storageVolumeResize: unsupported flags (0x4)

13/16. virsh: use option aliases
--tunneled can work well for migrate, but --read-bytes_sec doesn't work for blkdeviotune.

# virsh blkdeviotune  --read-bytes_sec
error: command 'blkdeviotune' doesn't support option --read-bytes_sec

# virsh blkdeviotune  --read-bytes-sec
error: expected syntax: --read-bytes-sec <number>

14/16. virsh: add command aliases, and rename nodedev-detach
show both detach command, saw that it's fixed on upstream but existing in rhel.

# virsh -h|grep nodedev-det
 Node Device (help keyword 'nodedev')
    nodedev-detach                 detach node device from its device driver
    nodedev-dettach                detach node device from its device driver


Others are okay for me:

6/16 xml: output memory unit for clarity

# virsh dumpxml rhel6 |grep -i mem

<memory unit='KiB'>2048576</memory>
  <currentMemory unit='KiB'>2049024</currentMemory>
  <memtune>
    <hard_limit unit='KiB'>512000</hard_limit>
    <soft_limit unit='KiB'>128000</soft_limit>
    <swap_hard_limit unit='KiB'>1024000</swap_hard_limit>
  </memtune>


1/16 blockResize: add flag for bytes
15/16  virsh: improve storage unit parsing

# virsh blockresize rhel62 --path /var/lib/libvirt/images/rhel62.img --size 5GB
Block device '/var/lib/libvirt/images/rhel62.img' is resized
# qemu-img info /var/lib/libvirt/images/rhel62.img
image: /var/lib/libvirt/images/rhel62.img
file format: raw
virtual size: 4.7G (5000000000 bytes)
disk size: 935M

# virsh blockresize rhel62 --path /var/lib/libvirt/images/rhel62.img --size 5GiB
Block device '/var/lib/libvirt/images/rhel62.img' is resized
# qemu-img info  /var/lib/libvirt/images/rhel62.img
image: /var/lib/libvirt/images/rhel62.img
file format: raw
virtual size: 5.0G (5368709120 bytes)
disk size: 864M

# virsh vol-resize /var/lib/libvirt/images/rhel5.img 7G
Size of volume 'rhel5.img' successfully changed to 7G

# qemu-img info /var/lib/libvirt/images/rhel5.img
image: /var/lib/libvirt/images/rhel5.img
file format: raw
virtual size: 7.0G (7516192768 bytes)
disk size: 0

# virsh vol-resize /var/lib/libvirt/images/rhel5.img 7GB
Size of volume 'rhel5.img' successfully changed to 7GB

# qemu-img info /var/lib/libvirt/images/rhel5.img
image: /var/lib/libvirt/images/rhel5.img
file format: raw
virtual size: 6.5G (7000000000 bytes)
disk size: 0


# virsh vol-resize /var/lib/libvirt/images/rhel5.img 1GiB --delta
Size of volume 'rhel5.img' successfully changed by 1GiB

# qemu-img info /var/lib/libvirt/images/rhel5.img
image: /var/lib/libvirt/images/rhel5.img
file format: raw
virtual size: 7.5G (8073741824 bytes)
disk size: 0


11/16  xml: allow scaled memory on input
16/16  virsh: improve memory unit parsing

# virsh setmaxmem rhel55 4G

# virsh dumpxml rhel55 |grep -i mem
  <memory unit='KiB'>4194304</memory>
  <currentMemory unit='KiB'>1048576</currentMemory>

# virsh setmaxmem rhel55 4GB

# virsh dumpxml rhel55 |grep -i mem
  <memory unit='KiB'>3906250</memory>
  <currentMemory unit='KiB'>1048576</currentMemory>

# virsh dumpxml rhel55|grep -i memtune -A 3
  <memtune>
    <hard_limit unit='KiB'>512000</hard_limit>
    <soft_limit unit='KiB'>128000</soft_limit>
    <swap_hard_limit unit='KiB'>1024000</swap_hard_limit>
  </memtune>

# virsh memtune rhel55 --hard-limit 512M --soft-limit 512MB --swap-hard-limit 512MiB

# virsh dumpxml rhel55|grep -i memtune -A 3
  <memtune>
    <hard_limit unit='KiB'>524288</hard_limit>
    <soft_limit unit='KiB'>500000</soft_limit>
    <swap_hard_limit unit='KiB'>524288</swap_hard_limit>
  </memtune>

# virsh setmem rhel55 --size=512M

# virsh dumpxml rhel55|grep -i mem
  <memory unit='KiB'>1048576</memory>
  <currentMemory unit='KiB'>524288</currentMemory>

# virsh setmem rhel55 --size=1G

[root@rhel62-release schemas]# virsh dumpxml rhel55|grep -i mem
  <memory unit='KiB'>1048576</memory>
  <currentMemory unit='KiB'>1048576</currentMemory>

# virsh setmem rhel55 --size=1GB

# virsh dumpxml rhel55|grep -i mem
  <memory unit='KiB'>1048576</memory>
  <currentMemory unit='KiB'>976564</currentMemory>

# virsh setmem rhel55 --size=1.5GB
error: memory size has to be a number

(In reply to comment #12)
> There are some issues in the following test:
> 
> 15/16. error msg for --shrink option of vol-resize
> # virsh vol-resize /var/lib/libvirt/images/rhel5.img 6000000000  --shrink
> error: Failed to change size of volume 'rhel5.img' to 6000000000
> 
> error: invalid argument: storageVolumeResize: unsupported flags (0x4)

Expected, and unrelated to this BZ - although the API allows --shrink, it has not yet been wired up and backported to RHEL.

> 
> 13/16. virsh: use option aliases
> --tunneled can work well for migrate, but --read-bytes_sec doesn't work for
> blkdeviotune.
> 
> # virsh blkdeviotune  --read-bytes_sec
> error: command 'blkdeviotune' doesn't support option --read-bytes_sec

Invalid, so the error is expected.  You can only use '--read-bytes-sec' (new preferred spelling) or '--read_bytes_sec' (deprecated but still supported spelling), but not a mix.

> 
> # virsh blkdeviotune  --read-bytes-sec
> error: expected syntax: --read-bytes-sec <number>

Invalid, so the error is expected.  You must supply an argument for --read-bytes-sec.

> 
> 14/16. virsh: add command aliases, and rename nodedev-detach
> show both detach command, saw that it's fixed on upstream but existing in rhel.
> 
> # virsh -h|grep nodedev-det
>  Node Device (help keyword 'nodedev')
>     nodedev-detach                 detach node device from its device driver
>     nodedev-dettach                detach node device from its device driver

Real bug; I fixed 'virsh help' but not 'virsh -h' to omit deprecated aliases.  I'll move this back to assigned; it should be a quick fix.

> Others are okay for me:

Good to hear.

> 
> 1/16 blockResize: add flag for bytes
> 15/16  virsh: improve storage unit parsing
> 
> # virsh blockresize rhel62 --path /var/lib/libvirt/images/rhel62.img --size 5GB

> # virsh blockresize rhel62 --path /var/lib/libvirt/images/rhel62.img --size
> 5GiB

Both of these blockresize commands happened to test a multiple of 1024 bytes, and therefore, while they tested the new virsh parsing of 15/16, they did not test the new bytes flag for 1/16.  You also want to test setting the size to something that is not a multiple of 1024.

> 
> # virsh setmem rhel55 --size=1.5GB
> error: memory size has to be a number

It might be nice to teach virsh to support fractional values, but I agree that the patch as-is did not do this, so this error is expected.

The fix for the one identified issue in QA testing is now pending upstream review:
https://www.redhat.com/archives/libvir-list/2012-March/msg00741.html

In POST:
http://post-office.corp.redhat.com/archives/rhvirt-patches/2012-March/msg01522.html

(In reply to comment #13)
> (In reply to comment #12)
> > There are some issues in the following test:
> > 
> > 15/16. error msg for --shrink option of vol-resize
> > # virsh vol-resize /var/lib/libvirt/images/rhel5.img 6000000000  --shrink
> > error: Failed to change size of volume 'rhel5.img' to 6000000000
> > 
> > error: invalid argument: storageVolumeResize: unsupported flags (0x4)
> 
> Expected, and unrelated to this BZ - although the API allows --shrink, it has
> not yet been wired up and backported to RHEL.
>
No bug to track it in rhel, will fill a new one to rhel6.4.0.

> > 
> > 13/16. virsh: use option aliases
> > --tunneled can work well for migrate, but --read-bytes_sec doesn't work for
> > blkdeviotune.
> > 
> > # virsh blkdeviotune  --read-bytes_sec
> > error: command 'blkdeviotune' doesn't support option --read-bytes_sec
> 
> Invalid, so the error is expected.  You can only use '--read-bytes-sec' (new
> preferred spelling) or '--read_bytes_sec' (deprecated but still supported
> spelling), but not a mix.
> 
Sorry, I read it wrong in the patch.

> > 
> > # virsh blkdeviotune  --read-bytes-sec
> > error: expected syntax: --read-bytes-sec <number>
> 
> Invalid, so the error is expected.  You must supply an argument for
> --read-bytes-sec.
> 
> > 
> > 14/16. virsh: add command aliases, and rename nodedev-detach
> > show both detach command, saw that it's fixed on upstream but existing in rhel.
> > 
> > # virsh -h|grep nodedev-det
> >  Node Device (help keyword 'nodedev')
> >     nodedev-detach                 detach node device from its device driver
> >     nodedev-dettach                detach node device from its device driver
> 
> Real bug; I fixed 'virsh help' but not 'virsh -h' to omit deprecated aliases. 
> I'll move this back to assigned; it should be a quick fix.
> 
> > Others are okay for me:
> 
> Good to hear.
> 
> > 
> > 1/16 blockResize: add flag for bytes
> > 15/16  virsh: improve storage unit parsing
> > 
> > # virsh blockresize rhel62 --path /var/lib/libvirt/images/rhel62.img --size 5GB
> 
> > # virsh blockresize rhel62 --path /var/lib/libvirt/images/rhel62.img --size
> > 5GiB
> 
> Both of these blockresize commands happened to test a multiple of 1024 bytes,
> and therefore, while they tested the new virsh parsing of 15/16, they did not
> test the new bytes flag for 1/16.  You also want to test setting the size to
> something that is not a multiple of 1024.
> 

Tested it with size < 1024bytes, got the unmatched size with qemu-img.
Seems no issue related to libvirt when setting to sth not a multiple of 1024, will re-check it for qemu-img.

# virsh blockresize rhel62 --path /var/lib/libvirt/images/rhel62.img --size 900B
Block device '/var/lib/libvirt/images/rhel62.img' is resized

# qemu-img info /var/lib/libvirt/images/rhel62.img
image: /var/lib/libvirt/images/rhel62.img
file format: raw
virtual size: 512 (512 bytes)
disk size: 4.0K

# ll /var/lib/libvirt/images/rhel62.img -h
-rw-------. 1 qemu qemu 900 Mar 19 11:11 /var/lib/libvirt/images/rhel62.img

# virsh blockresize rhel62 --path /var/lib/libvirt/images/rhel62.img --size 1024B
Block device '/var/lib/libvirt/images/rhel62.img' is resized

# qemu-img info /var/lib/libvirt/images/rhel62.img
image: /var/lib/libvirt/images/rhel62.img
file format: raw
virtual size: 1.0K (1024 bytes)
disk size: 4.0K

# virsh blockresize rhel62 --path /var/lib/libvirt/images/rhel62.img --size 600B
Block device '/var/lib/libvirt/images/rhel62.img' is resized
# qemu-img info /var/lib/libvirt/images/rhel62.img
image: /var/lib/libvirt/images/rhel62.img
file format: raw
virtual size: 512 (512 bytes)
disk size: 4.0K

# ll /var/lib/libvirt/images/rhel62.img -h
-rw-------. 1 qemu qemu 600 Mar 19 11:09 /var/lib/libvirt/images/rhel62.img


> > 
> > # virsh setmem rhel55 --size=1.5GB
> > error: memory size has to be a number
> 
> It might be nice to teach virsh to support fractional values, but I agree that
> the patch as-is did not do this, so this error is expected.

Verified the virsh -h issue PASS with libvirt-0.9.10-6.el6.

# virsh -h |grep nodedev-det
    nodedev-detach                 detach node device from its device driver

and new bug 804516 is to track the --shrink issue.

This patch introduced a regression tracked in bug 808522

See also bug 813972 for a related patch to the same area of code.

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
Previously, libvirt would output guest memory values in kibibytes (multiples of 1024), but with no indication of the scale.  Furthermore, the libvirt documentation referred to kilobytes (multiples of 1000).  Meanwhile, qemu defaults to mebibytes (multiples of 1024*1024), and these differences in scale could result in users making mistakes such as giving a guest 1000 times less memory than planned, with a failure mode that was not easy to diagnose.  Now the output is clear on the scale used, and the input allows user to use other scales that might be more convenient.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2012-0748.html