[libvirt] [PATCHv2 00/15] clean up unit handling of memory and storage

This turned into a bigger project than I originally planned, but I'm happy with how things look after this series. v1 (memory): https://www.redhat.com/archives/libvir-list/2012-February/msg00959.html v1 (virsh): https://www.redhat.com/archives/libvir-list/2012-March/msg00108.html In this series: Eric Blake (15): 1: new, but partially factored out of memory 3/3 docs: use correct terminology for 1024 bytes 2: new api: add overflow error 3: new, but partially factored out of memory 3/3 util: new function for scaling numbers 4: new, based on feedback of memory 1/3 xml: share 'unit' in RNG 5: rework of memory 1/3 xml: output memory unit for clarity 6: new storage: support more scaling suffixes 7: minor rework of memory 2/3 xml: drop unenforced minimum memory limit from RNG 8-9: new xml: use long long internally, to centralize overflow checks xml: use better types for memory values 10: rework of memory 3/3 xml: allow scaled memory on input 11-13: unchanged from virsh 1-3/3 virsh: add option aliases virsh: use option aliases virsh: add command aliases, and rename nodedev-detach 14-15: new virsh: improve storage unit parsing virsh: improve memory unit parsing Also available at: git fetch git://repo.or.cz/libvirt/ericb.git units or browseable at: http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/units daemon/remote.c | 17 +- docs/formatdomain.html.in | 55 +++- docs/formatstorage.html.in | 17 +- docs/schemas/basictypes.rng | 19 ++ docs/schemas/domaincommon.rng | 19 +- docs/schemas/storagepool.rng | 47 +-- docs/schemas/storagevol.rng | 56 +--- include/libvirt/virterror.h | 3 +- src/conf/domain_audit.c | 2 +- src/conf/domain_conf.c | 127 ++++++-- src/conf/domain_conf.h | 14 +- src/conf/storage_conf.c | 73 +---- src/conf/storage_conf.h | 12 +- src/driver.h | 2 +- src/esx/esx_driver.c | 4 +- src/libvirt.c | 47 ++- src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 8 +- src/lxc/lxc_driver.c | 6 +- src/openvz/openvz_driver.c | 8 +- src/phyp/phyp_driver.c | 10 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 14 +- src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_monitor_text.c | 2 +- src/qemu/qemu_monitor_text.h | 2 +- src/qemu/qemu_process.c | 6 + src/rpc/gendispatch.pl | 1 - src/test/test_driver.c | 4 +- src/uml/uml_conf.c | 2 +- src/uml/uml_driver.c | 6 +- src/util/util.c | 66 ++++ src/util/util.h | 4 + src/util/virterror.c | 6 + src/vbox/vbox_tmpl.c | 4 +- src/vmx/vmx.c | 10 +- src/xen/xen_driver.c | 6 +- src/xen/xen_hypervisor.c | 4 +- src/xen/xend_internal.c | 12 +- src/xen/xend_internal.h | 4 +- src/xen/xm_internal.c | 6 +- src/xen/xm_internal.h | 4 +- src/xen/xs_internal.c | 10 +- src/xen/xs_internal.h | 4 +- src/xenapi/xenapi_driver.c | 6 +- src/xenxs/xen_sxpr.c | 8 +- src/xenxs/xen_xm.c | 58 +++- tests/define-dev-segfault | 4 +- tests/domainschemadata/domain-lxc-simple.xml | 2 +- tests/domainschemadata/portprofile.xml | 2 +- .../qemu-simple-description-title.xml | 4 +- tests/domainschemadata/timers.xml | 4 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 4 +- tests/domainsnapshotxml2xmlout/full_domain.xml | 4 +- tests/domainsnapshotxml2xmlout/metadata.xml | 4 +- tests/openvzutilstest.c | 4 +- tests/qemuargv2xmltest.c | 5 +- .../qemuxml2argv-balloon-device-auto.xml | 4 +- .../qemuxml2argv-balloon-device.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-bios.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml | 4 +- .../qemuxml2argv-blkiotune-device.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-blkiotune.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml | 4 +- .../qemuxml2argv-boot-complex-bootindex.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-boot-complex.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-boot-floppy.xml | 4 +- ...uxml2argv-boot-menu-disable-drive-bootindex.xml | 4 +- .../qemuxml2argv-boot-menu-disable-drive.xml | 4 +- .../qemuxml2argv-boot-menu-disable.xml | 4 +- .../qemuxml2argv-boot-menu-enable.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-multi.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-boot-network.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml | 4 +- .../qemuxml2argv-channel-guestfwd.xml | 4 +- .../qemuxml2argv-channel-spicevmc-old.xml | 2 +- .../qemuxml2argv-channel-spicevmc.xml | 2 +- .../qemuxml2argv-channel-virtio-auto.xml | 4 +- .../qemuxml2argv-channel-virtio.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-clock-france.xml | 4 +- .../qemuxml2argv-clock-localtime.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml | 4 +- .../qemuxml2argv-clock-variable.xml | 4 +- .../qemuxml2argv-console-compat-auto.xml | 4 +- .../qemuxml2argv-console-compat-chardev.xml | 4 +- .../qemuxml2argv-console-compat.xml | 4 +- .../qemuxml2argv-console-virtio-many.xml | 4 +- .../qemuxml2argv-console-virtio.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml | 4 +- .../qemuxml2argv-cpu-exact2-nofallback.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml | 4 +- .../qemuxml2argv-cpu-host-kvmclock.xml | 4 +- .../qemuxml2argv-cpu-host-model-fallback.xml | 4 +- .../qemuxml2argv-cpu-host-model-nofallback.xml | 4 +- .../qemuxml2argv-cpu-host-model.xml | 4 +- .../qemuxml2argv-cpu-host-passthrough.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml | 4 +- .../qemuxml2argv-cpu-nofallback.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 4 +- .../qemuxml2argv-cpu-qemu-host-passthrough.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml | 4 +- .../qemuxml2argv-cpu-topology1.xml | 4 +- .../qemuxml2argv-cpu-topology2.xml | 4 +- .../qemuxml2argv-cpu-topology3.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cputune.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-aio.xml | 4 +- .../qemuxml2argv-disk-cdrom-empty.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml | 4 +- .../qemuxml2argv-disk-copy_on_read.xml | 2 +- .../qemuxml2argv-disk-drive-boot-cdrom.xml | 4 +- .../qemuxml2argv-disk-drive-boot-disk.xml | 4 +- .../qemuxml2argv-disk-drive-cache-directsync.xml | 4 +- .../qemuxml2argv-disk-drive-cache-unsafe.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v1-none.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v1-wb.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v1-wt.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v2-none.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v2-wb.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v2-wt.xml | 4 +- ...muxml2argv-disk-drive-error-policy-enospace.xml | 4 +- .../qemuxml2argv-disk-drive-error-policy-stop.xml | 4 +- ...rgv-disk-drive-error-policy-wreport-rignore.xml | 4 +- .../qemuxml2argv-disk-drive-fat.xml | 4 +- .../qemuxml2argv-disk-drive-fmt-qcow.xml | 4 +- .../qemuxml2argv-disk-drive-network-nbd.xml | 4 +- .../qemuxml2argv-disk-drive-network-rbd-auth.xml | 4 +- ...emuxml2argv-disk-drive-network-rbd-ceph-env.xml | 4 +- .../qemuxml2argv-disk-drive-network-rbd.xml | 4 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 4 +- .../qemuxml2argv-disk-drive-no-boot.xml | 4 +- .../qemuxml2argv-disk-drive-readonly-disk.xml | 4 +- .../qemuxml2argv-disk-drive-readonly-no-device.xml | 4 +- .../qemuxml2argv-disk-drive-shared.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-disk-floppy.xml | 4 +- .../qemuxml2argv-disk-ioeventfd.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-order.xml | 4 +- .../qemuxml2argv-disk-sata-device.xml | 4 +- .../qemuxml2argv-disk-scsi-device-auto.xml | 4 +- .../qemuxml2argv-disk-scsi-device.xml | 4 +- .../qemuxml2argv-disk-scsi-virtio-scsi.xml | 4 +- .../qemuxml2argv-disk-scsi-vscsi.xml | 4 +- .../qemuxml2argv-disk-snapshot.xml | 4 +- .../qemuxml2argv-disk-transient.xml | 4 +- .../qemuxml2argv-disk-usb-device.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-usb.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-disk-virtio.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml | 4 +- .../qemuxml2argv-encrypted-disk.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml | 4 +- .../qemuxml2argv-floppy-drive-fat.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml | 4 +- .../qemuxml2argv-graphics-listen-network.xml | 4 +- .../qemuxml2argv-graphics-listen-network2.xml | 4 +- .../qemuxml2argv-graphics-sdl-fullscreen.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 4 +- .../qemuxml2argv-graphics-spice-compression.xml | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.xml | 4 +- .../qemuxml2argv-graphics-spice-timeout.xml | 4 +- .../qemuxml2argv-graphics-spice.xml | 4 +- .../qemuxml2argv-graphics-vnc-sasl.xml | 4 +- .../qemuxml2argv-graphics-vnc-socket.xml | 4 +- .../qemuxml2argv-graphics-vnc-tls.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 4 +- .../qemuxml2argv-hostdev-pci-address-device.xml | 4 +- .../qemuxml2argv-hostdev-pci-address.xml | 4 +- .../qemuxml2argv-hostdev-usb-address-device.xml | 4 +- .../qemuxml2argv-hostdev-usb-address.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-hugepages.xml | 4 +- .../qemuxml2argv-input-usbmouse-addr.xml | 4 +- .../qemuxml2argv-input-usbmouse.xml | 4 +- .../qemuxml2argv-input-usbtablet.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-lease.xml | 4 +- .../qemuxml2argv-machine-aliases1.xml | 4 +- .../qemuxml2argv-machine-aliases2.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-memtune.xml | 10 +- tests/qemuxml2argvdata/qemuxml2argv-metadata.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-migrate.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-minimal.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.xml | 4 +- .../qemuxml2argv-misc-no-reboot.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-monitor-json.xml | 4 +- .../qemuxml2argv-multifunction-pci-device.xml | 4 +- .../qemuxml2argv-net-bandwidth.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-client.xml | 4 +- .../qemuxml2argv-net-eth-ifname.xml | 4 +- .../qemuxml2argv-net-eth-names.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-mcast.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-server.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 4 +- .../qemuxml2argv-net-virtio-device.xml | 4 +- .../qemuxml2argv-net-virtio-netdev.xml | 4 +- .../qemuxml2argv-net-virtio-network-portgroup.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-virtio.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-no-shutdown.xml | 4 +- .../qemuxml2argv-nographics-vga.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-nographics.xml | 4 +- .../qemuxml2argv-numatune-memory.xml | 4 +- .../qemuxml2argv-parallel-tcp-chardev.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-parallel-tcp.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml | 4 +- .../qemuxml2argv-pseries-basic.xml | 2 +- .../qemuxml2argv-pseries-vio-address-clash.xml | 2 +- .../qemuxml2argv-pseries-vio-user-assigned.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-vio.xml | 2 +- .../qemuxml2argv-qemu-ns-no-env.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-restore-v1.xml | 4 +- .../qemuxml2argv-restore-v2-fd.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-restore-v2.xml | 4 +- .../qemuxml2argv-seclabel-dynamic-baselabel.xml | 4 +- .../qemuxml2argv-seclabel-dynamic-override.xml | 4 +- .../qemuxml2argv-seclabel-dynamic.xml | 4 +- .../qemuxml2argv-seclabel-none.xml | 4 +- .../qemuxml2argv-seclabel-static-relabel.xml | 4 +- .../qemuxml2argv-seclabel-static.xml | 4 +- .../qemuxml2argv-serial-dev-chardev.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-dev.xml | 4 +- .../qemuxml2argv-serial-file-chardev.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-serial-file.xml | 4 +- .../qemuxml2argv-serial-many-chardev.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-serial-many.xml | 4 +- .../qemuxml2argv-serial-pty-chardev.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-pty.xml | 4 +- .../qemuxml2argv-serial-target-port-auto.xml | 4 +- .../qemuxml2argv-serial-tcp-chardev.xml | 4 +- .../qemuxml2argv-serial-tcp-telnet-chardev.xml | 4 +- .../qemuxml2argv-serial-tcp-telnet.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.xml | 4 +- .../qemuxml2argv-serial-udp-chardev.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-udp.xml | 4 +- .../qemuxml2argv-serial-unix-chardev.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-serial-unix.xml | 4 +- .../qemuxml2argv-serial-vc-chardev.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-vc.xml | 4 +- .../qemuxml2argv-smartcard-controller.xml | 4 +- .../qemuxml2argv-smartcard-host-certificates.xml | 4 +- .../qemuxml2argv-smartcard-host.xml | 4 +- ...qemuxml2argv-smartcard-passthrough-spicevmc.xml | 4 +- .../qemuxml2argv-smartcard-passthrough-tcp.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-smp.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-sound-device.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-sound.xml | 4 +- .../qemuxml2argv-usb-controller.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml | 4 +- .../qemuxml2argv-usb-ich9-companion.xml | 4 +- .../qemuxml2argv-usb-ich9-ehci-addr.xml | 4 +- .../qemuxml2argv-usb-piix3-controller.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml | 4 +- .../qemuxml2argv-watchdog-device.xml | 4 +- .../qemuxml2argv-watchdog-dump.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-watchdog.xml | 4 +- .../qemuxml2xmlout-balloon-device-auto.xml | 4 +- .../qemuxml2xmlout-channel-virtio-auto.xml | 4 +- .../qemuxml2xmlout-console-compat-auto.xml | 4 +- .../qemuxml2xmlout-console-virtio.xml | 4 +- .../qemuxml2xmlout-disk-cdrom-empty.xml | 4 +- .../qemuxml2xmlout-disk-scsi-device-auto.xml | 4 +- .../qemuxml2xmlout-graphics-listen-network2.xml | 4 +- .../qemuxml2xmlout-graphics-spice-timeout.xml | 4 +- .../qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml | 31 ++ .../qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml | 4 +- .../qemuxml2xmlout-serial-target-port-auto.xml | 4 +- tests/qemuxml2xmltest.c | 2 +- .../qemuxmlns-qemu-ns-commandline-ns0.xml | 4 +- .../qemuxmlns-qemu-ns-commandline-ns1.xml | 4 +- .../qemuxmlns-qemu-ns-commandline.xml | 4 +- .../qemuxmlns-qemu-ns-domain-commandline-ns0.xml | 4 +- .../qemuxmlns-qemu-ns-domain-commandline.xml | 4 +- .../qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.xml | 4 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-boot-grub.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-bridge-ipaddr.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-curmem.xml | 4 +- .../sexpr2xml-disk-block-shareable.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-disk-block.xml | 4 +- .../sexpr2xml-disk-drv-blktap-qcow.xml | 4 +- .../sexpr2xml-disk-drv-blktap-raw.xml | 4 +- .../sexpr2xml-disk-drv-blktap2-raw.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-disk-file.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 4 +- .../sexpr2xml-fv-serial-dev-2-ports.xml | 4 +- .../sexpr2xml-fv-serial-dev-2nd-port.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 4 +- .../sexpr2xml-fv-serial-tcp-telnet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-v2.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-net-bridged.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-net-e1000.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-net-routed.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pci-devs.xml | 4 +- .../sexpr2xml-pv-bootloader-cmdline.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-bootloader.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-localtime.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-vcpus.xml | 4 +- .../sexpr2xml-pv-vfb-new-vncdisplay.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-vfb-orig.xml | 4 +- .../sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv.xml | 4 +- tests/storagepoolxml2xmlout/pool-dir.xml | 6 +- tests/storagepoolxml2xmlout/pool-disk.xml | 6 +- tests/storagepoolxml2xmlout/pool-fs.xml | 6 +- tests/storagepoolxml2xmlout/pool-iscsi-auth.xml | 6 +- .../storagepoolxml2xmlout/pool-iscsi-multiiqn.xml | 6 +- .../pool-iscsi-vendor-product.xml | 6 +- tests/storagepoolxml2xmlout/pool-iscsi.xml | 6 +- .../storagepoolxml2xmlout/pool-logical-create.xml | 6 +- tests/storagepoolxml2xmlout/pool-logical.xml | 6 +- tests/storagepoolxml2xmlout/pool-mpath.xml | 6 +- tests/storagepoolxml2xmlout/pool-netfs.xml | 6 +- tests/storagepoolxml2xmlout/pool-scsi.xml | 6 +- tests/storagevolxml2xmlin/vol-file-backing.xml | 4 +- tests/storagevolxml2xmlin/vol-file.xml | 4 +- tests/storagevolxml2xmlout/vol-file-backing.xml | 4 +- tests/storagevolxml2xmlout/vol-file.xml | 4 +- tests/storagevolxml2xmlout/vol-logical-backing.xml | 4 +- tests/storagevolxml2xmlout/vol-logical.xml | 4 +- tests/storagevolxml2xmlout/vol-partition.xml | 4 +- tests/storagevolxml2xmlout/vol-qcow2.xml | 4 +- tests/virshtest.c | 6 + tests/vmx2xmldata/vmx2xml-annotation.xml | 4 +- tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml | 4 +- tests/vmx2xmldata/vmx2xml-case-insensitive-2.xml | 4 +- tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-4.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml | 6 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-bridged.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-custom.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-e1000.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-generated.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-nat.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-other.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-static.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-vmxnet2.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-vpx.xml | 4 +- tests/vmx2xmldata/vmx2xml-floppy-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-floppy-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-graphics-vnc.xml | 4 +- tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-1.xml | 4 +- tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-2.xml | 4 +- tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-3.xml | 4 +- tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-4.xml | 4 +- tests/vmx2xmldata/vmx2xml-harddisk-ide-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-minimal-64bit.xml | 4 +- tests/vmx2xmldata/vmx2xml-minimal.xml | 4 +- tests/vmx2xmldata/vmx2xml-parallel-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-parallel-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-scsi-driver.xml | 4 +- tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml | 4 +- tests/vmx2xmldata/vmx2xml-serial-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-serial-file.xml | 4 +- .../vmx2xmldata/vmx2xml-serial-network-client.xml | 4 +- .../vmx2xmldata/vmx2xml-serial-network-server.xml | 4 +- tests/vmx2xmldata/vmx2xml-serial-pipe.xml | 4 +- tests/vmx2xmldata/vmx2xml-smbios.xml | 4 +- tests/vmx2xmldata/vmx2xml-svga.xml | 4 +- tests/vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml | 4 +- tests/vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml | 4 +- tests/xmconfigdata/sexpr2xml-pv-bootloader.xml | 4 +- tests/xmconfigdata/test-escape-paths.xml | 4 +- tests/xmconfigdata/test-fullvirt-force-hpet.xml | 4 +- tests/xmconfigdata/test-fullvirt-force-nohpet.xml | 4 +- tests/xmconfigdata/test-fullvirt-localtime.xml | 4 +- tests/xmconfigdata/test-fullvirt-net-ioemu.xml | 4 +- tests/xmconfigdata/test-fullvirt-net-netfront.xml | 4 +- tests/xmconfigdata/test-fullvirt-new-cdrom.xml | 4 +- tests/xmconfigdata/test-fullvirt-old-cdrom.xml | 4 +- tests/xmconfigdata/test-fullvirt-parallel-tcp.xml | 4 +- .../test-fullvirt-serial-dev-2-ports.xml | 4 +- .../test-fullvirt-serial-dev-2nd-port.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-file.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-null.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-pipe.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-pty.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-stdio.xml | 4 +- .../test-fullvirt-serial-tcp-telnet.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-tcp.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-udp.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-unix.xml | 4 +- tests/xmconfigdata/test-fullvirt-sound.xml | 4 +- tests/xmconfigdata/test-fullvirt-usbmouse.xml | 4 +- .../test-fullvirt-usbtablet-no-bus.xml | 4 +- tests/xmconfigdata/test-fullvirt-usbtablet.xml | 4 +- tests/xmconfigdata/test-fullvirt-utc.xml | 4 +- tests/xmconfigdata/test-no-source-cdrom.xml | 4 +- tests/xmconfigdata/test-paravirt-net-e1000.xml | 4 +- tests/xmconfigdata/test-paravirt-net-vifname.xml | 4 +- .../test-paravirt-new-pvfb-vncdisplay.xml | 4 +- tests/xmconfigdata/test-paravirt-new-pvfb.xml | 4 +- .../test-paravirt-old-pvfb-vncdisplay.xml | 4 +- tests/xmconfigdata/test-paravirt-old-pvfb.xml | 4 +- tests/xmconfigdata/test-paravirt-vcpu.xml | 4 +- tests/xmconfigdata/test-pci-devs.xml | 4 +- tests/xml2sexprdata/xml2sexpr-boot-grub.xml | 2 +- tests/xml2sexprdata/xml2sexpr-bridge-ipaddr.xml | 2 +- tests/xml2sexprdata/xml2sexpr-curmem.xml | 4 +- .../xml2sexpr-disk-block-shareable.xml | 4 +- tests/xml2sexprdata/xml2sexpr-disk-block.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-drv-blkback.xml | 2 +- .../xml2sexpr-disk-drv-blktap-qcow.xml | 2 +- .../xml2sexpr-disk-drv-blktap-raw.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-drv-blktap.xml | 2 +- .../xml2sexpr-disk-drv-blktap2-raw.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-drv-blktap2.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-drv-loop.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-file.xml | 2 +- tests/xml2sexprdata/xml2sexpr-escape.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-force-hpet.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-force-nohpet.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-kernel.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-localtime.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-net-ioemu.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-net-netfront.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-parallel-tcp.xml | 2 +- .../xml2sexpr-fv-serial-dev-2-ports.xml | 2 +- .../xml2sexpr-fv-serial-dev-2nd-port.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-file.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-null.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-pipe.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-pty.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-stdio.xml | 2 +- .../xml2sexpr-fv-serial-tcp-telnet.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-tcp.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-udp.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-unix.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-sound.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-usbmouse.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-usbtablet.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-utc.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-vncunused.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv.xml | 2 +- tests/xml2sexprdata/xml2sexpr-net-bridged.xml | 2 +- tests/xml2sexprdata/xml2sexpr-net-e1000.xml | 2 +- tests/xml2sexprdata/xml2sexpr-net-routed.xml | 2 +- tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml | 4 +- tests/xml2sexprdata/xml2sexpr-pci-devs.xml | 2 +- .../xml2sexpr-pv-bootloader-cmdline.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-bootloader.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-localtime.xml | 4 +- tests/xml2sexprdata/xml2sexpr-pv-vcpus.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-vfb-new-auto.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-vfb-new.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-vfb-orig.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv.xml | 2 +- tests/xml2vmxdata/xml2vmx-annotation.xml | 2 +- tests/xml2vmxdata/xml2vmx-cdrom-ide-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-cdrom-ide-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-cdrom-scsi-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-cdrom-scsi-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-1.xml | 4 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-2.xml | 4 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-3.xml | 4 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-4.xml | 4 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.xml | 6 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml | 4 +- tests/xml2vmxdata/xml2vmx-ethernet-bridged.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-custom.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-e1000.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-generated.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-other.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-static.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-vmxnet2.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-vpx.xml | 2 +- tests/xml2vmxdata/xml2vmx-floppy-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-floppy-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-graphics-vnc.xml | 2 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-1.xml | 4 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-2.xml | 4 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-3.xml | 4 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-4.xml | 4 +- tests/xml2vmxdata/xml2vmx-harddisk-ide-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-harddisk-scsi-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-minimal-64bit.xml | 2 +- tests/xml2vmxdata/xml2vmx-minimal.xml | 2 +- tests/xml2vmxdata/xml2vmx-parallel-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-parallel-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-scsi-driver.xml | 2 +- tests/xml2vmxdata/xml2vmx-scsi-writethrough.xml | 2 +- tests/xml2vmxdata/xml2vmx-serial-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-serial-file.xml | 2 +- .../xml2vmxdata/xml2vmx-serial-network-client.xml | 2 +- .../xml2vmxdata/xml2vmx-serial-network-server.xml | 2 +- tests/xml2vmxdata/xml2vmx-serial-pipe.xml | 2 +- tests/xml2vmxdata/xml2vmx-smbios.xml | 2 +- tests/xml2vmxdata/xml2vmx-svga.xml | 2 +- tools/virsh.c | 330 ++++++++++++-------- tools/virsh.pod | 119 +++++--- 534 files changed, 1689 insertions(+), 1380 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml -- 1.7.7.6

Yes, I like kilobytes better than kibibytes (when I say kilobytes, I generally mean 1024). But since the term is ambiguous, it can't hurt to say what we mean, by using both the correct name and calling out the numeric equivalent. * src/libvirt.c (virDomainGetMaxMemory, virDomainSetMaxMemory) (virDomainSetMemory, virDomainSetMemoryFlags) (virNodeGetFreeMemory): Tweak wording. * docs/formatdomain.html.in: Likewise. * docs/formatstorage.html.in: Likewise. --- v2: separate doc changes out early docs/formatdomain.html.in | 15 ++++++++------- docs/formatstorage.html.in | 8 ++++++-- src/libvirt.c | 10 +++++----- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a61895f..0855c7f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -424,7 +424,7 @@ <dl> <dt><code>memory</code></dt> <dd>The maximum allocation of memory for the guest at boot time. - The units for this value are kilobytes (i.e. blocks of 1024 bytes)</dd> + The units for this value are kibibytes (i.e. blocks of 1024 bytes)</dd> <dt><code>currentMemory</code></dt> <dd>The actual allocation of memory for the guest. This value can be less than the maximum allocation, to allow for ballooning @@ -480,21 +480,21 @@ determine so one needs guess and try.</dd> <dt><code>hard_limit</code></dt> <dd> The optional <code>hard_limit</code> element is the maximum memory - the guest can use. The units for this value are kilobytes (i.e. blocks + the guest can use. The units for this value are kibibytes (i.e. blocks of 1024 bytes)</dd> <dt><code>soft_limit</code></dt> <dd> The optional <code>soft_limit</code> element is the memory limit to enforce during memory contention. The units for this value are - kilobytes (i.e. blocks of 1024 bytes)</dd> + kibibytes (i.e. blocks of 1024 bytes)</dd> <dt><code>swap_hard_limit</code></dt> <dd> The optional <code>swap_hard_limit</code> element is the maximum memory plus swap the guest can use. The units for this value are - kilobytes (i.e. blocks of 1024 bytes). This has to be more than + kibibytes (i.e. blocks of 1024 bytes). This has to be more than hard_limit value provided</dd> <dt><code>min_guarantee</code></dt> <dd> The optional <code>min_guarantee</code> element is the guaranteed minimum memory allocation for the guest. The units for this value are - kilobytes (i.e. blocks of 1024 bytes)</dd> + kibibytes (i.e. blocks of 1024 bytes)</dd> </dl> @@ -774,7 +774,7 @@ <p> Each <code>cell</code> element specifies a NUMA cell or a NUMA node. <code>cpus</code> specifies the CPU or range of CPUs that are part of - the node. <code>memory</code> specifies the node memory in kilobytes + the node. <code>memory</code> specifies the node memory in kibibytes (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid or nodeid in the increasing order starting from 0. </p> @@ -2920,7 +2920,8 @@ qemu-kvm -net nic,model=? /dev/null attribute which takes the value "vga", "cirrus", "vmvga", "xen", "vbox", or "qxl" (<span class="since">since 0.8.6</span>) depending on the hypervisor features available. - You can also provide the amount of video memory in kilobytes using + You can also provide the amount of video memory in kibibytes + (blocks of 1024 bytes) using <code>vram</code> and the number of screen with <code>heads</code>. </dd> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 0dcf6df..93d8ab2 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -236,8 +236,12 @@ <br/> By default this is specified in bytes, but an optional <code>unit</code> can be specified to adjust the passed value. - Values can be: 'K' (kilobytes), 'M' (megabytes), 'G' (gigabytes), - 'T' (terabytes), 'P' (petabytes), or 'E' (exabytes). + Values can be: 'K' (kibibytes, 2<sup>10</sup> or 1024), 'M' + (mebibytes, 2<sup>20</sup> or 1,048,576), 'G' (gibibytes, + 2<sup>30</sup> or 1,073,741,824), 'T' (tebibytes, + 2<sup>40</sup> or 1,099,511,627,776), 'P' (pebibytes, + 2<sup>50</sup> or 1,125,899,906,842,624), or 'E' (exbibytes, + 2<sup>60</sup> or 1,152,921,504,606,846,976). <span class="since">Since 0.4.1</span></dd> <dt><code>capacity</code></dt> <dd>Providing the logical capacity for the volume. This value is diff --git a/src/libvirt.c b/src/libvirt.c index d98741b..650252e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3582,7 +3582,7 @@ error: * domain. If domain is NULL, then this get the amount of memory reserved * to Domain0 i.e. the domain where the application runs. * - * Returns the memory size in kilobytes or 0 in case of error. + * Returns the memory size in kibibytes (blocks of 1024), or 0 in case of error. */ unsigned long virDomainGetMaxMemory(virDomainPtr domain) @@ -3619,7 +3619,7 @@ error: /** * virDomainSetMaxMemory: * @domain: a domain object or NULL - * @memory: the memory size in kilobytes + * @memory: the memory size in kibibytes (blocks of 1024) * * Dynamically change the maximum amount of physical memory allocated to a * domain. If domain is NULL, then this change the amount of memory reserved @@ -3674,7 +3674,7 @@ error: /** * virDomainSetMemory: * @domain: a domain object or NULL - * @memory: the memory size in kilobytes + * @memory: the memory size in kibibytes (blocks of 1024) * * Dynamically change the target amount of physical memory allocated to a * domain. If domain is NULL, then this change the amount of memory reserved @@ -3729,7 +3729,7 @@ error: /** * virDomainSetMemoryFlags: * @domain: a domain object or NULL - * @memory: the memory size in kilobytes + * @memory: the memory size in kibibytes (blocks of 1024) * @flags: bitwise-OR of virDomainMemoryModFlags * * Dynamically change the target amount of physical memory allocated to a @@ -6684,7 +6684,7 @@ error: * @conn: pointer to the hypervisor connection * * provides the free memory available on the Node - * Note: most libvirt APIs provide memory sizes in kilobytes, but in this + * Note: most libvirt APIs provide memory sizes in kibibytes, but in this * function the returned value is in bytes. Divide by 1024 as necessary. * * Returns the available free memory in bytes or 0 in case of error -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Yes, I like kilobytes better than kibibytes (when I say kilobytes, I generally mean 1024). But since the term is ambiguous, it can't hurt to say what we mean, by using both the correct name and calling out the numeric equivalent.
* src/libvirt.c (virDomainGetMaxMemory, virDomainSetMaxMemory) (virDomainSetMemory, virDomainSetMemoryFlags) (virNodeGetFreeMemory): Tweak wording. * docs/formatdomain.html.in: Likewise. * docs/formatstorage.html.in: Likewise. ---
v2: separate doc changes out early
docs/formatdomain.html.in | 15 ++++++++------- docs/formatstorage.html.in | 8 ++++++-- src/libvirt.c | 10 +++++----- 3 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 0dcf6df..93d8ab2 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -236,8 +236,12 @@ <br/> By default this is specified in bytes, but an optional <code>unit</code> can be specified to adjust the passed value. - Values can be: 'K' (kilobytes), 'M' (megabytes), 'G' (gigabytes), - 'T' (terabytes), 'P' (petabytes), or 'E' (exabytes). + Values can be: 'K' (kibibytes, 2<sup>10</sup> or 1024), 'M'
I'd probably write this as (kibibytes, 2<sup>10</sup> or 1024 bytes) to be a little more specific, but it should be clear enough without it.
+ (mebibytes, 2<sup>20</sup> or 1,048,576), 'G' (gibibytes, + 2<sup>30</sup> or 1,073,741,824), 'T' (tebibytes, + 2<sup>40</sup> or 1,099,511,627,776), 'P' (pebibytes, + 2<sup>50</sup> or 1,125,899,906,842,624), or 'E' (exbibytes, + 2<sup>60</sup> or 1,152,921,504,606,846,976). <span class="since">Since 0.4.1</span></dd> <dt><code>capacity</code></dt> <dd>Providing the logical capacity for the volume. This value is
ACK, Peter

On Mon, Mar 05, 2012 at 05:34:16PM -0700, Eric Blake wrote:
@@ -3619,7 +3619,7 @@ error: /** * virDomainSetMaxMemory: * @domain: a domain object or NULL - * @memory: the memory size in kilobytes + * @memory: the memory size in kibibytes (blocks of 1024)
"of 1024 bytes"? I don't know how this looks to a native speaker, but this looks truncated to my French eyes if "bytes" is not there, same for the 2 other occurrences below. Christophe
* * Dynamically change the maximum amount of physical memory allocated to a * domain. If domain is NULL, then this change the amount of memory reserved @@ -3674,7 +3674,7 @@ error: /** * virDomainSetMemory: * @domain: a domain object or NULL - * @memory: the memory size in kilobytes + * @memory: the memory size in kibibytes (blocks of 1024) * * Dynamically change the target amount of physical memory allocated to a * domain. If domain is NULL, then this change the amount of memory reserved @@ -3729,7 +3729,7 @@ error: /** * virDomainSetMemoryFlags: * @domain: a domain object or NULL - * @memory: the memory size in kilobytes + * @memory: the memory size in kibibytes (blocks of 1024) * @flags: bitwise-OR of virDomainMemoryModFlags * * Dynamically change the target amount of physical memory allocated to a @@ -6684,7 +6684,7 @@ error: * @conn: pointer to the hypervisor connection * * provides the free memory available on the Node - * Note: most libvirt APIs provide memory sizes in kilobytes, but in this + * Note: most libvirt APIs provide memory sizes in kibibytes, but in this * function the returned value is in bytes. Divide by 1024 as necessary. * * Returns the available free memory in bytes or 0 in case of error -- 1.7.7.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 03/06/2012 03:34 PM, Christophe Fergeau wrote:
On Mon, Mar 05, 2012 at 05:34:16PM -0700, Eric Blake wrote:
@@ -3619,7 +3619,7 @@ error: /** * virDomainSetMaxMemory: * @domain: a domain object or NULL - * @memory: the memory size in kilobytes + * @memory: the memory size in kibibytes (blocks of 1024)
"of 1024 bytes"? I don't know how this looks to a native speaker, but this looks truncated to my French eyes if "bytes" is not there, same for the 2 other occurrences below.
Christophe
Oh yeah, my geeky brain inferred the bytes part automaticaly when it saw the number 1024. Adding the "bytes" makes it more clear for not-so-geeky people :). Eric, could you please add "bytes" to the appropriate places to make it more clear. Peter

Overflow can be user-induced, so it deserves more than being called an internal error. Note that in general, 32-bit platforms have far more places to trigger this error (anywhere the public API used 'unsigned long' but the other side of the connection is a 64-bit server); but some are possible on 64-bit platforms (where the public API computes the product of two numbers). * include/libvirt/virterror.h (VIR_ERR_OVERFLOW): New error. * src/util/virterror.c (virErrorMsg): Translate it. * src/libvirt.c (virDomainSetVcpusFlags, virDomainGetVcpuPinInfo) (virDomainGetVcpus, virDomainGetCPUStats): Use it. * daemon/remote.c (HYPER_TO_TYPE): Likewise. * src/qemu/qemu_driver.c (qemuDomainBlockResize): Likewise. --- v2: new daemon/remote.c | 17 +++++++++-------- include/libvirt/virterror.h | 3 ++- src/libvirt.c | 30 +++++++++++++++++++++++------- src/qemu/qemu_driver.c | 2 +- src/util/virterror.c | 6 ++++++ 5 files changed, 41 insertions(+), 17 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 74a5f16..be51d6f 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -57,14 +57,15 @@ __FUNCTION__, __LINE__, __VA_ARGS__) #if SIZEOF_LONG < 8 -# define HYPER_TO_TYPE(_type, _to, _from) \ - do { \ - if ((_from) != (_type)(_from)) { \ - virNetError(VIR_ERR_INTERNAL_ERROR, \ - _("conversion from hyper to %s overflowed"), #_type); \ - goto cleanup; \ - } \ - (_to) = (_from); \ +# define HYPER_TO_TYPE(_type, _to, _from) \ + do { \ + if ((_from) != (_type)(_from)) { \ + virNetError(VIR_ERR_OVERFLOW, \ + _("conversion from hyper to %s overflowed"), \ + #_type); \ + goto cleanup; \ + } \ + (_to) = (_from); \ } while (0) # define HYPER_TO_LONG(_to, _from) HYPER_TO_TYPE(long, _to, _from) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index a3f9199..38ac15e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -4,7 +4,7 @@ * Description: Provides the interfaces of the libvirt library to handle * errors raised while using the library. * - * Copy: Copyright (C) 2006, 2010-2011 Red Hat, Inc. + * Copy: Copyright (C) 2006, 2010-2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -246,6 +246,7 @@ typedef enum { VIR_ERR_AUTH_CANCELLED = 79, /* authentication cancelled */ VIR_ERR_NO_DOMAIN_METADATA = 80, /* The metadata is not present */ VIR_ERR_MIGRATE_UNSAFE = 81, /* Migration is not safe */ + VIR_ERR_OVERFLOW = 82, /* integer overflow */ } virErrorNumber; /** diff --git a/src/libvirt.c b/src/libvirt.c index 650252e..de23db8 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8506,10 +8506,14 @@ virDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, } /* Perform some argument validation common to all implementations. */ - if (nvcpus < 1 || (unsigned short) nvcpus != nvcpus) { + if (nvcpus < 1) { virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } + if ((unsigned short) nvcpus != nvcpus) { + virLibDomainError(VIR_ERR_OVERFLOW, _("input too large: %u"), nvcpus); + goto error; + } conn = domain->conn; if (conn->driver->domainSetVcpusFlags) { @@ -8773,11 +8777,15 @@ virDomainGetVcpuPinInfo(virDomainPtr domain, int ncpumaps, return -1; } - if (ncpumaps < 1 || !cpumaps || maplen <= 0 || - INT_MULTIPLY_OVERFLOW(ncpumaps, maplen)) { + if (ncpumaps < 1 || !cpumaps || maplen <= 0) { virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } + if (INT_MULTIPLY_OVERFLOW(ncpumaps, maplen)) { + virLibDomainError(VIR_ERR_OVERFLOW, _("input too large: %d * %d"), + ncpumaps, maplen); + goto error; + } /* At most one of these two flags should be set. */ if ((flags & VIR_DOMAIN_AFFECT_LIVE) && @@ -8852,11 +8860,15 @@ virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, /* Ensure that domainGetVcpus (aka remoteDomainGetVcpus) does not try to memcpy anything into a NULL pointer. */ - if (!cpumaps ? maplen != 0 - : (maplen <= 0 || INT_MULTIPLY_OVERFLOW(maxinfo, maplen))) { + if (!cpumaps ? maplen != 0 : maplen <= 0) { virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } + if (cpumaps && INT_MULTIPLY_OVERFLOW(maxinfo, maplen)) { + virLibDomainError(VIR_ERR_OVERFLOW, _("input too large: %d * %d"), + maxinfo, maplen); + goto error; + } conn = domain->conn; @@ -18613,11 +18625,15 @@ int virDomainGetCPUStats(virDomainPtr domain, if (start_cpu < -1 || (start_cpu == -1 && ncpus != 1) || ((params == NULL) != (nparams == 0)) || - (ncpus == 0 && params != NULL) || - (nparams && ncpus > UINT_MAX / nparams)) { + (ncpus == 0 && params != NULL)) { virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } + if (nparams && ncpus > UINT_MAX / nparams) { + virLibDomainError(VIR_ERR_OVERFLOW, _("input too large: %u * %u"), + nparams, ncpus); + goto error; + } if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_TYPED_PARAM_STRING)) flags |= VIR_TYPED_PARAM_STRING_OKAY; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 733df0a..1bdd2f0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7371,7 +7371,7 @@ qemuDomainBlockResize(virDomainPtr dom, /* We prefer operating on bytes. */ if ((flags & VIR_DOMAIN_BLOCK_RESIZE_BYTES) == 0) { if (size > ULLONG_MAX / 1024) { - qemuReportError(VIR_ERR_INVALID_ARG, + qemuReportError(VIR_ERR_OVERFLOW, _("size must be less than %llu"), ULLONG_MAX / 1024); return -1; diff --git a/src/util/virterror.c b/src/util/virterror.c index 48c61a2..b4e496a 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1238,6 +1238,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("Unsafe migration: %s"); break; + case VIR_ERR_OVERFLOW: + if (!info) + errmsg = _("numerical overflow"); + else + errmsg = _("numerical overflow: %s"); + break; } return (errmsg); } -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Overflow can be user-induced, so it deserves more than being called an internal error. Note that in general, 32-bit platforms have
Definitely. Internal error is always somewhat scary to users.
far more places to trigger this error (anywhere the public API used 'unsigned long' but the other side of the connection is a 64-bit server); but some are possible on 64-bit platforms (where the public API computes the product of two numbers).
* include/libvirt/virterror.h (VIR_ERR_OVERFLOW): New error. * src/util/virterror.c (virErrorMsg): Translate it. * src/libvirt.c (virDomainSetVcpusFlags, virDomainGetVcpuPinInfo) (virDomainGetVcpus, virDomainGetCPUStats): Use it. * daemon/remote.c (HYPER_TO_TYPE): Likewise. * src/qemu/qemu_driver.c (qemuDomainBlockResize): Likewise. ---
ACK, Peter

Scaling an integer based on a suffix is something we plan on reusing in several contexts: XML parsing, virsh CLI parsing, and possibly elsewhere. Make it easy to reuse, as well as adding in support for powers of 1000. * src/util/util.h (virScaleInteger): New function. * src/util/util.c (virScaleInteger): Implement it. * src/libvirt_private.syms (util.h): Export it. --- v2: new, but borrows ideas from memory v1 3/3 src/libvirt_private.syms | 1 + src/util/util.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/util.h | 4 +++ 3 files changed, 71 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a104e70..a6d053b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1121,6 +1121,7 @@ virKillProcess; virParseNumber; virParseVersionString; virPipeReadUntilEOF; +virScaleInteger; virSetBlocking; virSetCloseExec; virSetInherit; diff --git a/src/util/util.c b/src/util/util.c index 7c58c7b..1b71680 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1632,6 +1632,72 @@ virHexToBin(unsigned char c) } } +/* Scale an integer VALUE by an optional SUFFIX in-place, defaulting + * to SCALE if no suffix is present. Ensure that the result does not + * exceed LIMIT. Return 0 on success, -1 with error message raised on + * failure. */ +int +virScaleInteger(unsigned long long *value, const char *suffix, + unsigned long long scale, unsigned long long limit) +{ + if (!suffix || !*suffix) { + if (!scale) { + virUtilError(VIR_ERR_INTERNAL_ERROR, + _("invalid scale %llu"), scale); + return -1; + } + suffix = ""; + } else if (STRCASEEQ(suffix, "b") || STRCASEEQ(suffix, "byte") || + STRCASEEQ(suffix, "bytes")) { + scale = 1; + } else { + int base; + + if (!suffix[1] || STRCASEEQ(suffix + 1, "iB")) { + base = 1024; + } else if (c_tolower(suffix[1]) == 'b' && !suffix[2]) { + base = 1000; + } else { + virUtilError(VIR_ERR_INVALID_ARG, + _("unknown suffix '%s'"), suffix); + return -1; + } + scale = 1; + switch (c_tolower(*suffix)) { + case 'e': + scale *= base; + /* fallthrough */ + case 'p': + scale *= base; + /* fallthrough */ + case 't': + scale *= base; + /* fallthrough */ + case 'g': + scale *= base; + /* fallthrough */ + case 'm': + scale *= base; + /* fallthrough */ + case 'k': + scale *= base; + break; + default: + virUtilError(VIR_ERR_INVALID_ARG, + _("unknown suffix '%s'"), suffix); + return -1; + } + } + + if (*value >= (limit / scale)) { + virUtilError(VIR_ERR_OVERFLOW, _("value too large: %llu%s"), + *value, suffix); + return -1; + } + *value *= scale; + return 0; +} + /** * virSkipSpaces: * @str: pointer to the char pointer used diff --git a/src/util/util.h b/src/util/util.h index 5c945cc..85e8bd6 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -157,6 +157,10 @@ int virStrToDouble(char const *s, char **end_ptr, double *result); +int virScaleInteger(unsigned long long *value, const char *suffix, + unsigned long long scale, unsigned long long limit) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + int virHexToBin(unsigned char c); void virSkipSpaces(const char **str) ATTRIBUTE_NONNULL(1); -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Scaling an integer based on a suffix is something we plan on reusing in several contexts: XML parsing, virsh CLI parsing, and possibly elsewhere. Make it easy to reuse, as well as adding in support for powers of 1000.
* src/util/util.h (virScaleInteger): New function. * src/util/util.c (virScaleInteger): Implement it. * src/libvirt_private.syms (util.h): Export it. ---
v2: new, but borrows ideas from memory v1 3/3
src/libvirt_private.syms | 1 + src/util/util.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/util.h | 4 +++ 3 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c index 7c58c7b..1b71680 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1632,6 +1632,72 @@ virHexToBin(unsigned char c) } }
+/* Scale an integer VALUE by an optional SUFFIX in-place, defaulting + * to SCALE if no suffix is present. Ensure that the result does not + * exceed LIMIT. Return 0 on success, -1 with error message raised on + * failure. */
I'd write a little bit more on how the base selection works depending on the argument. Something like: For power-of-two scaling use the binary prefixes (i.e. KiB, MiB), for power of ten scaling use the SI prefixes (i.e. KB, Mb, ...).
+int +virScaleInteger(unsigned long long *value, const char *suffix, + unsigned long long scale, unsigned long long limit)
Looks good. ACK Peter

The code supported unit='E' for "exabyte", but the RNG did not; conversely, the RNG supported "z" and "y" but the code did not (I'm jealous if you have that much storage, particularly since it won't fit in 64-bit off_t). Also, the code supported <allocation unit='...'>, but not the RNG. In an effort to make 'unit' more worthwhile in future patches, it's easier to share it between files. In making this factorization, note that absFilePath is more permissive than 'path', so storage pools and storage volumes will now validate with a wider set of file names than before. I don't think this should be a problem in practice. * docs/schemas/storagepool.rng: Include basic types, rather than repeating things here. * docs/schemas/storagevol.rng: Likewise. * docs/schemas/basictypes.rng: Add 'unsignedLong', 'unit', and fix to match storage code. --- v2: new docs/schemas/basictypes.rng | 11 ++++++++ docs/schemas/storagepool.rng | 47 +++++++++------------------------- docs/schemas/storagevol.rng | 56 ++++++++++++----------------------------- 3 files changed, 41 insertions(+), 73 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 3b4b952..4f16fa7 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -8,6 +8,11 @@ <param name="pattern">[0-9]+</param> </data> </define> + <define name='unsignedLong'> + <data type='unsignedLong'> + <param name='pattern'>[0-9]+</param> + </data> + </define> <define name="positiveInteger"> <data type="positiveInteger"> @@ -133,4 +138,10 @@ </data> </define> + <define name='unit'> + <data type='string'> + <param name='pattern'>[kKmMgGtTpPeE]</param> + </data> + </define> + </grammar> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 3301472..6399f12 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -2,6 +2,7 @@ <!-- A Relax NG schema for the libvirt storage pool XML format --> <grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <include href='basictypes.rng'/> <start> <ref name='pool'/> </start> @@ -127,7 +128,7 @@ </element> <optional> <element name='uuid'> - <ref name='uuid'/> + <ref name='UUID'/> </element> </optional> </define> @@ -135,17 +136,17 @@ <define name='sizing'> <optional> <element name='capacity'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> </optional> <optional> <element name='allocation'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> </optional> <optional> <element name='available'> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> </optional> </define> @@ -154,13 +155,13 @@ <optional> <element name='permissions'> <element name='mode'> - <ref name='uint'/> + <ref name='unsignedInt'/> </element> <element name='owner'> - <ref name='uint'/> + <ref name='unsignedInt'/> </element> <element name='group'> - <ref name='uint'/> + <ref name='unsignedInt'/> </element> <optional> <element name='label'> @@ -175,7 +176,7 @@ <element name='target'> <optional> <element name='path'> - <ref name='path'/> + <ref name='absFilePath'/> </element> </optional> <ref name='permissions'/> @@ -200,7 +201,7 @@ <element name='device'> <attribute name='path'> <choice> - <ref name='path'/> + <ref name='absFilePath'/> <ref name='name'/> </choice> </attribute> @@ -226,10 +227,10 @@ <oneOrMore> <element name='freeExtent'> <attribute name='start'> - <ref name='uint'/> + <ref name='unsignedLong'/> </attribute> <attribute name='end'> - <ref name='uint'/> + <ref name='unsignedLong'/> </attribute> </element> </oneOrMore> @@ -238,7 +239,7 @@ <define name='sourceinfodir'> <element name='dir'> <attribute name='path'> - <ref name='path'/> + <ref name='absFilePath'/> </attribute> <empty/> </element> @@ -454,28 +455,6 @@ <param name="pattern">[a-zA-Z0-9_\+\-]+</param> </data> </define> - <define name='uuid'> - <choice> - <data type='string'> - <param name="pattern">[a-fA-F0-9]{32}</param> - </data> - <data type='string'> - <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> - </data> - </choice> - </define> - - <define name='uint'> - <data type='string'> - <param name="pattern">[0-9]+</param> - </data> - </define> - - <define name='path'> - <data type='string'> - <param name="pattern">/[a-zA-Z0-9_\+\-/%]+</param> - </data> - </define> <define name="PortNumber"> <data type="short"> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 994004d..d06884c 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -1,6 +1,8 @@ +<?xml version="1.0"?> <!-- A Relax NG schema for the libvirt storage volume XML format --> <grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <include href='basictypes.rng'/> <start> <ref name='vol'/> </start> @@ -35,12 +37,17 @@ <ref name='unit'/> </attribute> </optional> - <ref name='uint'/> + <ref name='unsignedLong'/> </element> </optional> <optional> <element name='allocation'> - <ref name='uint'/> + <optional> + <attribute name='unit'> + <ref name='unit'/> + </attribute> + </optional> + <ref name='unsignedLong'/> </element> </optional> </define> @@ -49,13 +56,13 @@ <optional> <element name='permissions'> <element name='mode'> - <ref name='uint'/> + <ref name='unsignedInt'/> </element> <element name='owner'> - <ref name='uint'/> + <ref name='unsignedInt'/> </element> <element name='group'> - <ref name='uint'/> + <ref name='unsignedInt'/> </element> <optional> <element name='label'> @@ -70,7 +77,7 @@ <element name='target'> <optional> <element name='path'> - <ref name='path'/> + <ref name='absFilePath'/> </element> </optional> <ref name='format'/> @@ -84,7 +91,7 @@ <define name='backingStore'> <element name='backingStore'> <element name='path'> - <ref name='path'/> + <ref name='absFilePath'/> </element> <ref name='format'/> <ref name='permissions'/> @@ -103,7 +110,7 @@ <define name='sourcedev'> <element name='device'> <attribute name='path'> - <ref name='path'/> + <ref name='absFilePath'/> </attribute> <choice> <empty/> @@ -116,10 +123,10 @@ <oneOrMore> <element name='extent'> <attribute name='start'> - <ref name='uint'/> + <ref name='unsignedLong'/> </attribute> <attribute name='end'> - <ref name='uint'/> + <ref name='unsignedLong'/> </attribute> </element> </oneOrMore> @@ -180,33 +187,4 @@ </data> </define> - <define name='uint'> - <data type='string'> - <param name="pattern">[0-9]+</param> - </data> - </define> - - <define name='path'> - <data type='string'> - <param name="pattern">/[a-zA-Z0-9_\+\-\./%]+</param> - </data> - </define> - - <define name='unit'> - <data type='string'> - <param name="pattern">[kKmMgGtTpPyYzZ]</param> - </data> - </define> - - <define name="UUID"> - <choice> - <data type="string"> - <param name="pattern">[a-fA-F0-9]{32}</param> - </data> - <data type="string"> - <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> - </data> - </choice> - </define> - </grammar> -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
The code supported unit='E' for "exabyte", but the RNG did not; conversely, the RNG supported "z" and "y" but the code did not (I'm jealous if you have that much storage, particularly since it won't fit in 64-bit off_t). Also, the code supported <allocation unit='...'>, but not the RNG.
In an effort to make 'unit' more worthwhile in future patches, it's easier to share it between files.
In making this factorization, note that absFilePath is more permissive than 'path', so storage pools and storage volumes will now validate with a wider set of file names than before. I don't think this should be a problem in practice.
* docs/schemas/storagepool.rng: Include basic types, rather than repeating things here. * docs/schemas/storagevol.rng: Likewise. * docs/schemas/basictypes.rng: Add 'unsignedLong', 'unit', and fix to match storage code. ---
There are more places that can be simplified including the basictypes.rng file. I'll post a separate patch for them. ACK, Peter

Make it obvious to 'dumpxml' readers what unit we are using, since our default of KiB for memory (1024) differs from qemu's default of MiB; while we use bytes for storage. Tests were updated via: $ find tests/*data tests/*out -name '*.xml' | \ xargs sed -i 's/<\(memory\|currentMemory\|hard_limit\|soft_limit\|min_guarantee\|swap_hard_limit\)>/<\1 unit='"'KiB'>/" $ find tests/*data tests/*out -name '*.xml' | \ xargs sed -i 's/<\(capacity\|allocation\|available\)>/<\1 unit='"'bytes'>/" followed by a few fixes for the stragglers. * docs/schemas/basictypes.rng (unit): Add 'bytes'. (scaledInteger): New define. * docs/schemas/storagevol.rng (sizing): Use it. * docs/schemas/storagepool.rng (sizing): Likewise. * docs/schemas/domaincommon.rng (memoryKBElement): New define; use for memory elements. * src/conf/storage_conf.c (virStoragePoolDefFormat) (virStorageVolDefFormat): Likewise. * src/conf/domain_conf.h (_virDomainDef): Document unit used internally. * src/conf/storage_conf.h (_virStoragePoolDef, _virStorageVolDef): Likewise. * tests/*data/*.xml: Update all tests. * tests/*out/*.xml: Likewise. * tests/define-dev-segfault: Likewise. * tests/openvzutilstest.c (testReadNetworkConf): Likewise. * tests/qemuargv2xmltest.c (blankProblemElements): Likewise. --- corresponds to memory v1 1/3; v2: also output units for storage, use 'unit=' not 'units=', use common RNG Portions of this patch elided to reduce mail size; see cover letter for git repo to see entire patch. docs/schemas/basictypes.rng | 10 +++++++- docs/schemas/domaincommon.rng | 25 +++++++++++++++---- docs/schemas/storagepool.rng | 6 ++-- docs/schemas/storagevol.rng | 14 +--------- src/conf/domain_conf.c | 21 ++++++++-------- src/conf/domain_conf.h | 12 ++++---- src/conf/storage_conf.c | 12 ++++---- src/conf/storage_conf.h | 12 ++++---- tests/define-dev-segfault | 4 +- tests/domainschemadata/domain-lxc-simple.xml | 2 +- tests/domainschemadata/portprofile.xml | 2 +- .../qemu-simple-description-title.xml | 4 +- tests/domainschemadata/timers.xml | 4 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 4 +- tests/domainsnapshotxml2xmlout/full_domain.xml | 4 +- tests/domainsnapshotxml2xmlout/metadata.xml | 4 +- tests/openvzutilstest.c | 4 +- tests/qemuargv2xmltest.c | 5 ++- .../qemuxml2argv-balloon-device-auto.xml | 4 +- .../qemuxml2argv-balloon-device.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-bios.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml | 4 +- .../qemuxml2argv-blkiotune-device.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-blkiotune.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml | 4 +- .../qemuxml2argv-boot-complex-bootindex.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-boot-complex.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-boot-floppy.xml | 4 +- ...uxml2argv-boot-menu-disable-drive-bootindex.xml | 4 +- .../qemuxml2argv-boot-menu-disable-drive.xml | 4 +- .../qemuxml2argv-boot-menu-disable.xml | 4 +- .../qemuxml2argv-boot-menu-enable.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-multi.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-boot-network.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml | 4 +- .../qemuxml2argv-channel-guestfwd.xml | 4 +- .../qemuxml2argv-channel-spicevmc-old.xml | 2 +- .../qemuxml2argv-channel-spicevmc.xml | 2 +- .../qemuxml2argv-channel-virtio-auto.xml | 4 +- .../qemuxml2argv-channel-virtio.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-clock-france.xml | 4 +- .../qemuxml2argv-clock-localtime.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml | 4 +- .../qemuxml2argv-clock-variable.xml | 4 +- .../qemuxml2argv-console-compat-auto.xml | 4 +- .../qemuxml2argv-console-compat-chardev.xml | 4 +- .../qemuxml2argv-console-compat.xml | 4 +- .../qemuxml2argv-console-virtio-many.xml | 4 +- .../qemuxml2argv-console-virtio.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml | 4 +- .../qemuxml2argv-cpu-exact2-nofallback.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml | 4 +- .../qemuxml2argv-cpu-host-kvmclock.xml | 4 +- .../qemuxml2argv-cpu-host-model-fallback.xml | 4 +- .../qemuxml2argv-cpu-host-model-nofallback.xml | 4 +- .../qemuxml2argv-cpu-host-model.xml | 4 +- .../qemuxml2argv-cpu-host-passthrough.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml | 4 +- .../qemuxml2argv-cpu-nofallback.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 4 +- .../qemuxml2argv-cpu-qemu-host-passthrough.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml | 4 +- .../qemuxml2argv-cpu-topology1.xml | 4 +- .../qemuxml2argv-cpu-topology2.xml | 4 +- .../qemuxml2argv-cpu-topology3.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cputune.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-aio.xml | 4 +- .../qemuxml2argv-disk-cdrom-empty.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml | 4 +- .../qemuxml2argv-disk-copy_on_read.xml | 2 +- .../qemuxml2argv-disk-drive-boot-cdrom.xml | 4 +- .../qemuxml2argv-disk-drive-boot-disk.xml | 4 +- .../qemuxml2argv-disk-drive-cache-directsync.xml | 4 +- .../qemuxml2argv-disk-drive-cache-unsafe.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v1-none.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v1-wb.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v1-wt.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v2-none.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v2-wb.xml | 4 +- .../qemuxml2argv-disk-drive-cache-v2-wt.xml | 4 +- ...muxml2argv-disk-drive-error-policy-enospace.xml | 4 +- .../qemuxml2argv-disk-drive-error-policy-stop.xml | 4 +- ...rgv-disk-drive-error-policy-wreport-rignore.xml | 4 +- .../qemuxml2argv-disk-drive-fat.xml | 4 +- .../qemuxml2argv-disk-drive-fmt-qcow.xml | 4 +- .../qemuxml2argv-disk-drive-network-nbd.xml | 4 +- .../qemuxml2argv-disk-drive-network-rbd-auth.xml | 4 +- ...emuxml2argv-disk-drive-network-rbd-ceph-env.xml | 4 +- .../qemuxml2argv-disk-drive-network-rbd.xml | 4 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 4 +- .../qemuxml2argv-disk-drive-no-boot.xml | 4 +- .../qemuxml2argv-disk-drive-readonly-disk.xml | 4 +- .../qemuxml2argv-disk-drive-readonly-no-device.xml | 4 +- .../qemuxml2argv-disk-drive-shared.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-disk-floppy.xml | 4 +- .../qemuxml2argv-disk-ioeventfd.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-order.xml | 4 +- .../qemuxml2argv-disk-sata-device.xml | 4 +- .../qemuxml2argv-disk-scsi-device-auto.xml | 4 +- .../qemuxml2argv-disk-scsi-device.xml | 4 +- .../qemuxml2argv-disk-scsi-virtio-scsi.xml | 4 +- .../qemuxml2argv-disk-scsi-vscsi.xml | 4 +- .../qemuxml2argv-disk-snapshot.xml | 4 +- .../qemuxml2argv-disk-transient.xml | 4 +- .../qemuxml2argv-disk-usb-device.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-disk-usb.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-disk-virtio.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml | 4 +- .../qemuxml2argv-encrypted-disk.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml | 4 +- .../qemuxml2argv-floppy-drive-fat.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml | 4 +- .../qemuxml2argv-graphics-listen-network.xml | 4 +- .../qemuxml2argv-graphics-listen-network2.xml | 4 +- .../qemuxml2argv-graphics-sdl-fullscreen.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 4 +- .../qemuxml2argv-graphics-spice-compression.xml | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.xml | 4 +- .../qemuxml2argv-graphics-spice-timeout.xml | 4 +- .../qemuxml2argv-graphics-spice.xml | 4 +- .../qemuxml2argv-graphics-vnc-sasl.xml | 4 +- .../qemuxml2argv-graphics-vnc-socket.xml | 4 +- .../qemuxml2argv-graphics-vnc-tls.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 4 +- .../qemuxml2argv-hostdev-pci-address-device.xml | 4 +- .../qemuxml2argv-hostdev-pci-address.xml | 4 +- .../qemuxml2argv-hostdev-usb-address-device.xml | 4 +- .../qemuxml2argv-hostdev-usb-address.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-hugepages.xml | 4 +- .../qemuxml2argv-input-usbmouse-addr.xml | 4 +- .../qemuxml2argv-input-usbmouse.xml | 4 +- .../qemuxml2argv-input-usbtablet.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-lease.xml | 4 +- .../qemuxml2argv-machine-aliases1.xml | 4 +- .../qemuxml2argv-machine-aliases2.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-memtune.xml | 10 ++++---- tests/qemuxml2argvdata/qemuxml2argv-metadata.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-migrate.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-minimal.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.xml | 4 +- .../qemuxml2argv-misc-no-reboot.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-monitor-json.xml | 4 +- .../qemuxml2argv-multifunction-pci-device.xml | 4 +- .../qemuxml2argv-net-bandwidth.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-client.xml | 4 +- .../qemuxml2argv-net-eth-ifname.xml | 4 +- .../qemuxml2argv-net-eth-names.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-mcast.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-server.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 4 +- .../qemuxml2argv-net-virtio-device.xml | 4 +- .../qemuxml2argv-net-virtio-netdev.xml | 4 +- .../qemuxml2argv-net-virtio-network-portgroup.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-virtio.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-no-shutdown.xml | 4 +- .../qemuxml2argv-nographics-vga.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-nographics.xml | 4 +- .../qemuxml2argv-numatune-memory.xml | 4 +- .../qemuxml2argv-parallel-tcp-chardev.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-parallel-tcp.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml | 4 +- .../qemuxml2argv-pseries-basic.xml | 2 +- .../qemuxml2argv-pseries-vio-address-clash.xml | 2 +- .../qemuxml2argv-pseries-vio-user-assigned.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-vio.xml | 2 +- .../qemuxml2argv-qemu-ns-no-env.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-restore-v1.xml | 4 +- .../qemuxml2argv-restore-v2-fd.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-restore-v2.xml | 4 +- .../qemuxml2argv-seclabel-dynamic-baselabel.xml | 4 +- .../qemuxml2argv-seclabel-dynamic-override.xml | 4 +- .../qemuxml2argv-seclabel-dynamic.xml | 4 +- .../qemuxml2argv-seclabel-none.xml | 4 +- .../qemuxml2argv-seclabel-static-relabel.xml | 4 +- .../qemuxml2argv-seclabel-static.xml | 4 +- .../qemuxml2argv-serial-dev-chardev.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-dev.xml | 4 +- .../qemuxml2argv-serial-file-chardev.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-serial-file.xml | 4 +- .../qemuxml2argv-serial-many-chardev.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-serial-many.xml | 4 +- .../qemuxml2argv-serial-pty-chardev.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-pty.xml | 4 +- .../qemuxml2argv-serial-target-port-auto.xml | 4 +- .../qemuxml2argv-serial-tcp-chardev.xml | 4 +- .../qemuxml2argv-serial-tcp-telnet-chardev.xml | 4 +- .../qemuxml2argv-serial-tcp-telnet.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.xml | 4 +- .../qemuxml2argv-serial-udp-chardev.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-udp.xml | 4 +- .../qemuxml2argv-serial-unix-chardev.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-serial-unix.xml | 4 +- .../qemuxml2argv-serial-vc-chardev.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-serial-vc.xml | 4 +- .../qemuxml2argv-smartcard-controller.xml | 4 +- .../qemuxml2argv-smartcard-host-certificates.xml | 4 +- .../qemuxml2argv-smartcard-host.xml | 4 +- ...qemuxml2argv-smartcard-passthrough-spicevmc.xml | 4 +- .../qemuxml2argv-smartcard-passthrough-tcp.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-smp.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-sound-device.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-sound.xml | 4 +- .../qemuxml2argv-usb-controller.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml | 4 +- .../qemuxml2argv-usb-ich9-companion.xml | 4 +- .../qemuxml2argv-usb-ich9-ehci-addr.xml | 4 +- .../qemuxml2argv-usb-piix3-controller.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml | 4 +- .../qemuxml2argv-watchdog-device.xml | 4 +- .../qemuxml2argv-watchdog-dump.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-watchdog.xml | 4 +- .../qemuxml2xmlout-balloon-device-auto.xml | 4 +- .../qemuxml2xmlout-channel-virtio-auto.xml | 4 +- .../qemuxml2xmlout-console-compat-auto.xml | 4 +- .../qemuxml2xmlout-console-virtio.xml | 4 +- .../qemuxml2xmlout-disk-cdrom-empty.xml | 4 +- .../qemuxml2xmlout-disk-scsi-device-auto.xml | 4 +- .../qemuxml2xmlout-graphics-listen-network2.xml | 4 +- .../qemuxml2xmlout-graphics-spice-timeout.xml | 4 +- .../qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml | 4 +- .../qemuxml2xmlout-serial-target-port-auto.xml | 4 +- .../qemuxmlns-qemu-ns-commandline-ns0.xml | 4 +- .../qemuxmlns-qemu-ns-commandline-ns1.xml | 4 +- .../qemuxmlns-qemu-ns-commandline.xml | 4 +- .../qemuxmlns-qemu-ns-domain-commandline-ns0.xml | 4 +- .../qemuxmlns-qemu-ns-domain-commandline.xml | 4 +- .../qemuxmlnsdata/qemuxmlns-qemu-ns-domain-ns0.xml | 4 +- tests/qemuxmlnsdata/qemuxmlns-qemu-ns-domain.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-boot-grub.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-bridge-ipaddr.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-curmem.xml | 4 +- .../sexpr2xml-disk-block-shareable.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-disk-block.xml | 4 +- .../sexpr2xml-disk-drv-blktap-qcow.xml | 4 +- .../sexpr2xml-disk-drv-blktap-raw.xml | 4 +- .../sexpr2xml-disk-drv-blktap2-raw.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-disk-file.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 4 +- .../sexpr2xml-fv-serial-dev-2-ports.xml | 4 +- .../sexpr2xml-fv-serial-dev-2nd-port.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 4 +- .../sexpr2xml-fv-serial-tcp-telnet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-v2.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-net-bridged.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-net-e1000.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-net-routed.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pci-devs.xml | 4 +- .../sexpr2xml-pv-bootloader-cmdline.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-bootloader.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-localtime.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-vcpus.xml | 4 +- .../sexpr2xml-pv-vfb-new-vncdisplay.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-vfb-orig.xml | 4 +- .../sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv.xml | 4 +- tests/storagepoolxml2xmlout/pool-dir.xml | 6 ++-- tests/storagepoolxml2xmlout/pool-disk.xml | 6 ++-- tests/storagepoolxml2xmlout/pool-fs.xml | 6 ++-- tests/storagepoolxml2xmlout/pool-iscsi-auth.xml | 6 ++-- .../storagepoolxml2xmlout/pool-iscsi-multiiqn.xml | 6 ++-- .../pool-iscsi-vendor-product.xml | 6 ++-- tests/storagepoolxml2xmlout/pool-iscsi.xml | 6 ++-- .../storagepoolxml2xmlout/pool-logical-create.xml | 6 ++-- tests/storagepoolxml2xmlout/pool-logical.xml | 6 ++-- tests/storagepoolxml2xmlout/pool-mpath.xml | 6 ++-- tests/storagepoolxml2xmlout/pool-netfs.xml | 6 ++-- tests/storagepoolxml2xmlout/pool-scsi.xml | 6 ++-- tests/storagevolxml2xmlout/vol-file-backing.xml | 4 +- tests/storagevolxml2xmlout/vol-file.xml | 4 +- tests/storagevolxml2xmlout/vol-logical-backing.xml | 4 +- tests/storagevolxml2xmlout/vol-logical.xml | 4 +- tests/storagevolxml2xmlout/vol-partition.xml | 4 +- tests/storagevolxml2xmlout/vol-qcow2.xml | 4 +- tests/vmx2xmldata/vmx2xml-annotation.xml | 4 +- tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml | 4 +- tests/vmx2xmldata/vmx2xml-case-insensitive-2.xml | 4 +- tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-4.xml | 4 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml | 6 ++-- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-bridged.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-custom.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-e1000.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-generated.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-nat.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-other.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-static.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-vmxnet2.xml | 4 +- tests/vmx2xmldata/vmx2xml-ethernet-vpx.xml | 4 +- tests/vmx2xmldata/vmx2xml-floppy-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-floppy-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-graphics-vnc.xml | 4 +- tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-1.xml | 4 +- tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-2.xml | 4 +- tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-3.xml | 4 +- tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-4.xml | 4 +- tests/vmx2xmldata/vmx2xml-harddisk-ide-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-minimal-64bit.xml | 4 +- tests/vmx2xmldata/vmx2xml-minimal.xml | 4 +- tests/vmx2xmldata/vmx2xml-parallel-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-parallel-file.xml | 4 +- tests/vmx2xmldata/vmx2xml-scsi-driver.xml | 4 +- tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml | 4 +- tests/vmx2xmldata/vmx2xml-serial-device.xml | 4 +- tests/vmx2xmldata/vmx2xml-serial-file.xml | 4 +- .../vmx2xmldata/vmx2xml-serial-network-client.xml | 4 +- .../vmx2xmldata/vmx2xml-serial-network-server.xml | 4 +- tests/vmx2xmldata/vmx2xml-serial-pipe.xml | 4 +- tests/vmx2xmldata/vmx2xml-smbios.xml | 4 +- tests/vmx2xmldata/vmx2xml-svga.xml | 4 +- tests/vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml | 4 +- tests/vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml | 4 +- tests/xmconfigdata/sexpr2xml-pv-bootloader.xml | 4 +- tests/xmconfigdata/test-escape-paths.xml | 4 +- tests/xmconfigdata/test-fullvirt-force-hpet.xml | 4 +- tests/xmconfigdata/test-fullvirt-force-nohpet.xml | 4 +- tests/xmconfigdata/test-fullvirt-localtime.xml | 4 +- tests/xmconfigdata/test-fullvirt-net-ioemu.xml | 4 +- tests/xmconfigdata/test-fullvirt-net-netfront.xml | 4 +- tests/xmconfigdata/test-fullvirt-new-cdrom.xml | 4 +- tests/xmconfigdata/test-fullvirt-old-cdrom.xml | 4 +- tests/xmconfigdata/test-fullvirt-parallel-tcp.xml | 4 +- .../test-fullvirt-serial-dev-2-ports.xml | 4 +- .../test-fullvirt-serial-dev-2nd-port.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-file.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-null.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-pipe.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-pty.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-stdio.xml | 4 +- .../test-fullvirt-serial-tcp-telnet.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-tcp.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-udp.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-unix.xml | 4 +- tests/xmconfigdata/test-fullvirt-sound.xml | 4 +- tests/xmconfigdata/test-fullvirt-usbmouse.xml | 4 +- .../test-fullvirt-usbtablet-no-bus.xml | 4 +- tests/xmconfigdata/test-fullvirt-usbtablet.xml | 4 +- tests/xmconfigdata/test-fullvirt-utc.xml | 4 +- tests/xmconfigdata/test-no-source-cdrom.xml | 4 +- tests/xmconfigdata/test-paravirt-net-e1000.xml | 4 +- tests/xmconfigdata/test-paravirt-net-vifname.xml | 4 +- .../test-paravirt-new-pvfb-vncdisplay.xml | 4 +- tests/xmconfigdata/test-paravirt-new-pvfb.xml | 4 +- .../test-paravirt-old-pvfb-vncdisplay.xml | 4 +- tests/xmconfigdata/test-paravirt-old-pvfb.xml | 4 +- tests/xmconfigdata/test-paravirt-vcpu.xml | 4 +- tests/xmconfigdata/test-pci-devs.xml | 4 +- tests/xml2sexprdata/xml2sexpr-boot-grub.xml | 2 +- tests/xml2sexprdata/xml2sexpr-bridge-ipaddr.xml | 2 +- tests/xml2sexprdata/xml2sexpr-curmem.xml | 4 +- .../xml2sexpr-disk-block-shareable.xml | 4 +- tests/xml2sexprdata/xml2sexpr-disk-block.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-drv-blkback.xml | 2 +- .../xml2sexpr-disk-drv-blktap-qcow.xml | 2 +- .../xml2sexpr-disk-drv-blktap-raw.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-drv-blktap.xml | 2 +- .../xml2sexpr-disk-drv-blktap2-raw.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-drv-blktap2.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-drv-loop.xml | 2 +- tests/xml2sexprdata/xml2sexpr-disk-file.xml | 2 +- tests/xml2sexprdata/xml2sexpr-escape.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-force-hpet.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-force-nohpet.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-kernel.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-localtime.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-net-ioemu.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-net-netfront.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-parallel-tcp.xml | 2 +- .../xml2sexpr-fv-serial-dev-2-ports.xml | 2 +- .../xml2sexpr-fv-serial-dev-2nd-port.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-file.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-null.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-pipe.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-pty.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-stdio.xml | 2 +- .../xml2sexpr-fv-serial-tcp-telnet.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-tcp.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-udp.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-serial-unix.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-sound.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-usbmouse.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-usbtablet.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-utc.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv-vncunused.xml | 2 +- tests/xml2sexprdata/xml2sexpr-fv.xml | 2 +- tests/xml2sexprdata/xml2sexpr-net-bridged.xml | 2 +- tests/xml2sexprdata/xml2sexpr-net-e1000.xml | 2 +- tests/xml2sexprdata/xml2sexpr-net-routed.xml | 2 +- tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml | 4 +- tests/xml2sexprdata/xml2sexpr-pci-devs.xml | 2 +- .../xml2sexpr-pv-bootloader-cmdline.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-bootloader.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-localtime.xml | 4 +- tests/xml2sexprdata/xml2sexpr-pv-vcpus.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-vfb-new-auto.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-vfb-new.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv-vfb-orig.xml | 2 +- tests/xml2sexprdata/xml2sexpr-pv.xml | 2 +- tests/xml2vmxdata/xml2vmx-annotation.xml | 2 +- tests/xml2vmxdata/xml2vmx-cdrom-ide-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-cdrom-ide-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-cdrom-scsi-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-cdrom-scsi-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-1.xml | 4 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-2.xml | 4 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-3.xml | 4 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-4.xml | 4 +- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.xml | 6 ++-- tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml | 4 +- tests/xml2vmxdata/xml2vmx-ethernet-bridged.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-custom.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-e1000.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-generated.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-other.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-static.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-vmxnet2.xml | 2 +- tests/xml2vmxdata/xml2vmx-ethernet-vpx.xml | 2 +- tests/xml2vmxdata/xml2vmx-floppy-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-floppy-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-graphics-vnc.xml | 2 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-1.xml | 4 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-2.xml | 4 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-3.xml | 4 +- tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-4.xml | 4 +- tests/xml2vmxdata/xml2vmx-harddisk-ide-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-harddisk-scsi-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-minimal-64bit.xml | 2 +- tests/xml2vmxdata/xml2vmx-minimal.xml | 2 +- tests/xml2vmxdata/xml2vmx-parallel-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-parallel-file.xml | 2 +- tests/xml2vmxdata/xml2vmx-scsi-driver.xml | 2 +- tests/xml2vmxdata/xml2vmx-scsi-writethrough.xml | 2 +- tests/xml2vmxdata/xml2vmx-serial-device.xml | 2 +- tests/xml2vmxdata/xml2vmx-serial-file.xml | 2 +- .../xml2vmxdata/xml2vmx-serial-network-client.xml | 2 +- .../xml2vmxdata/xml2vmx-serial-network-server.xml | 2 +- tests/xml2vmxdata/xml2vmx-serial-pipe.xml | 2 +- tests/xml2vmxdata/xml2vmx-smbios.xml | 2 +- tests/xml2vmxdata/xml2vmx-svga.xml | 2 +- 485 files changed, 946 insertions(+), 933 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 4f16fa7..a50349c 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -140,8 +140,16 @@ <define name='unit'> <data type='string'> - <param name='pattern'>[kKmMgGtTpPeE]</param> + <param name='pattern'>(bytes)|[kKmMgGtTpPeE]</param> </data> </define> + <define name='scaledInteger'> + <optional> + <attribute name='unit'> + <ref name='unit'/> + </attribute> + </optional> + <ref name='unsignedLong'/> + </define> </grammar> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3908733..c8b403e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -412,11 +412,11 @@ <define name="resources"> <interleave> <element name="memory"> - <ref name="memoryKB"/> + <ref name='memoryKBElement'/> </element> <optional> <element name="currentMemory"> - <ref name="memoryKB"/> + <ref name='memoryKBElement'/> </element> </optional> <optional> @@ -461,25 +461,25 @@ <!-- Maximum memory the VM can use --> <optional> <element name="hard_limit"> - <ref name="memoryKB"/> + <ref name='memoryKBElement'/> </element> </optional> <!-- Minimum memory ascertained for the VM during contention --> <optional> <element name="soft_limit"> - <ref name="memoryKB"/> + <ref name='memoryKBElement'/> </element> </optional> <!-- Minimum amount of memory required to start the VM --> <optional> <element name="min_guarantee"> - <ref name="memoryKB"/> + <ref name='memoryKBElement'/> </element> </optional> <!-- Maximum swap area the VM can use --> <optional> <element name="swap_hard_limit"> - <ref name="memoryKB"/> + <ref name='memoryKBElement'/> </element> </optional> </element> @@ -3079,12 +3079,25 @@ <param name="maxInclusive">1000</param> </data> </define> + <!-- Memory as an attribute is in KiB, no way to express a unit --> <define name="memoryKB"> <data type="unsignedInt"> <param name="pattern">[0-9]+</param> <param name="minInclusive">4000</param> </data> </define> + <!-- Memory as an element, with optional unit attribute --> + <define name='memoryKBElement'> + <optional> + <attribute name='unit'> + <value>KiB</value> + </attribute> + </optional> + <data type='unsignedInt'> + <param name='pattern'>[0-9]+</param> + <param name='minInclusive'>4000</param> + </data> + </define> <define name="domainName"> <data type="string"> <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 6399f12..d4c80da 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -136,17 +136,17 @@ <define name='sizing'> <optional> <element name='capacity'> - <ref name='unsignedLong'/> + <ref name='scaledInteger'/> </element> </optional> <optional> <element name='allocation'> - <ref name='unsignedLong'/> + <ref name='scaledInteger'/> </element> </optional> <optional> <element name='available'> - <ref name='unsignedLong'/> + <ref name='scaledInteger'/> </element> </optional> </define> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index d06884c..8edb877 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -32,22 +32,12 @@ <define name='sizing'> <optional> <element name='capacity'> - <optional> - <attribute name='unit'> - <ref name='unit'/> - </attribute> - </optional> - <ref name='unsignedLong'/> + <ref name='scaledInteger'/> </element> </optional> <optional> <element name='allocation'> - <optional> - <attribute name='unit'> - <ref name='unit'/> - </attribute> - </optional> - <ref name='unsignedLong'/> + <ref name='scaledInteger'/> </element> </optional> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f9654f1..331d923 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11638,8 +11638,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, xmlIndentTreeOutput = oldIndentTreeOutput; } - virBufferAsprintf(buf, " <memory>%lu</memory>\n", def->mem.max_balloon); - virBufferAsprintf(buf, " <currentMemory>%lu</currentMemory>\n", + virBufferAsprintf(buf, " <memory unit='KiB'>%lu</memory>\n", + def->mem.max_balloon); + virBufferAsprintf(buf, " <currentMemory unit='KiB'>%lu</currentMemory>\n", def->mem.cur_balloon); /* add blkiotune only if there are any */ @@ -11680,20 +11681,20 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->mem.swap_hard_limit) virBufferAddLit(buf, " <memtune>\n"); if (def->mem.hard_limit) { - virBufferAsprintf(buf, " <hard_limit>%lu</hard_limit>\n", - def->mem.hard_limit); + virBufferAsprintf(buf, " <hard_limit unit='KiB'>" + "%lu</hard_limit>\n", def->mem.hard_limit); } if (def->mem.soft_limit) { - virBufferAsprintf(buf, " <soft_limit>%lu</soft_limit>\n", - def->mem.soft_limit); + virBufferAsprintf(buf, " <soft_limit unit='KiB'>" + "%lu</soft_limit>\n", def->mem.soft_limit); } if (def->mem.min_guarantee) { - virBufferAsprintf(buf, " <min_guarantee>%lu</min_guarantee>\n", - def->mem.min_guarantee); + virBufferAsprintf(buf, " <min_guarantee unit='KiB'>" + "%lu</min_guarantee>\n", def->mem.min_guarantee); } if (def->mem.swap_hard_limit) { - virBufferAsprintf(buf, " <swap_hard_limit>%lu</swap_hard_limit>\n", - def->mem.swap_hard_limit); + virBufferAsprintf(buf, " <swap_hard_limit unit='KiB'>" + "%lu</swap_hard_limit>\n", def->mem.swap_hard_limit); } if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || def->mem.swap_hard_limit) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d98a551..2aa489d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1454,13 +1454,13 @@ struct _virDomainDef { } blkio; struct { - unsigned long max_balloon; - unsigned long cur_balloon; + unsigned long max_balloon; /* in kibibytes */ + unsigned long cur_balloon; /* in kibibytes */ unsigned long hugepage_backed; - unsigned long hard_limit; - unsigned long soft_limit; - unsigned long min_guarantee; - unsigned long swap_hard_limit; + unsigned long hard_limit; /* in kibibytes */ + unsigned long soft_limit; /* in kibibytes */ + unsigned long min_guarantee; /* in kibibytes */ + unsigned long swap_hard_limit; /* in kibibytes */ } mem; unsigned short vcpus; unsigned short maxvcpus; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 87f3d6a..07f3f5b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1,7 +1,7 @@ /* * storage_conf.c: config handling for storage driver * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -897,11 +897,11 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) { virUUIDFormat(def->uuid, uuid); virBufferAsprintf(&buf," <uuid>%s</uuid>\n", uuid); - virBufferAsprintf(&buf," <capacity>%llu</capacity>\n", + virBufferAsprintf(&buf," <capacity unit='bytes'>%llu</capacity>\n", def->capacity); - virBufferAsprintf(&buf," <allocation>%llu</allocation>\n", + virBufferAsprintf(&buf," <allocation unit='bytes'>%llu</allocation>\n", def->allocation); - virBufferAsprintf(&buf," <available>%llu</available>\n", + virBufferAsprintf(&buf," <available unit='bytes'>%llu</available>\n", def->available); if (virStoragePoolSourceFormat(&buf, options, &def->source) < 0) @@ -1262,9 +1262,9 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, } virBufferAddLit(&buf, " </source>\n"); - virBufferAsprintf(&buf," <capacity>%llu</capacity>\n", + virBufferAsprintf(&buf," <capacity unit='bytes'>%llu</capacity>\n", def->capacity); - virBufferAsprintf(&buf," <allocation>%llu</allocation>\n", + virBufferAsprintf(&buf," <allocation unit='bytes'>%llu</allocation>\n", def->allocation); if (virStorageVolTargetDefFormat(options, &buf, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index dfed0bf..1ef9295 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -1,7 +1,7 @@ /* * storage_conf.h: config handling for storage driver * - * Copyright (C) 2006-2008, 2010-2011 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010-2012 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -92,8 +92,8 @@ struct _virStorageVolDef { unsigned int building; - unsigned long long allocation; - unsigned long long capacity; + unsigned long long allocation; /* bytes */ + unsigned long long capacity; /* bytes */ virStorageVolSource source; virStorageVolTarget target; @@ -262,9 +262,9 @@ struct _virStoragePoolDef { unsigned char uuid[VIR_UUID_BUFLEN]; int type; /* virStoragePoolType */ - unsigned long long allocation; - unsigned long long capacity; - unsigned long long available; + unsigned long long allocation; /* bytes */ + unsigned long long capacity; /* bytes */ + unsigned long long available; /* bytes */ virStoragePoolSource source; virStoragePoolTarget target; diff --git a/tests/define-dev-segfault b/tests/define-dev-segfault index 0a0532b..0c37f6d 100755 --- a/tests/define-dev-segfault +++ b/tests/define-dev-segfault @@ -20,8 +20,8 @@ cat <<\EOF > D.xml || fail=1 <domain type='test'> <name>D</name> <uuid>aaa3ae22-fed2-bfbd-ac02-3bea3bcfad82</uuid> - <memory>262144</memory> - <currentMemory>262144</currentMemory> + <memory unit='KiB'>262144</memory> + <currentMemory unit='KiB'>262144</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/domainschemadata/domain-lxc-simple.xml b/tests/domainschemadata/domain-lxc-simple.xml index 653bbc7..e61434f 100644 --- a/tests/domainschemadata/domain-lxc-simple.xml +++ b/tests/domainschemadata/domain-lxc-simple.xml @@ -5,7 +5,7 @@ <type>exe</type> <init>/sh</init> </os> - <memory>500000</memory> + <memory unit='KiB'>500000</memory> <devices> <filesystem type='mount'> <source dir='/root/container'/> ... diff --git a/tests/domainsnapshotxml2xmlout/metadata.xml b/tests/domainsnapshotxml2xmlout/metadata.xml index 263cbba..1a27773 100644 --- a/tests/domainsnapshotxml2xmlout/metadata.xml +++ b/tests/domainsnapshotxml2xmlout/metadata.xml @@ -13,8 +13,8 @@ <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> </metadata> - <memory>219100</memory> - <currentMemory>219100</currentMemory> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> <vcpu cpuset='1-4,8-20,525'>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/openvzutilstest.c b/tests/openvzutilstest.c index 7f5ddc9..05121fe 100644 --- a/tests/openvzutilstest.c +++ b/tests/openvzutilstest.c @@ -78,8 +78,8 @@ testReadNetworkConf(const void *data ATTRIBUTE_UNUSED) const char *expected = "<domain type='openvz'>\n" " <uuid>00000000-0000-0000-0000-000000000000</uuid>\n" - " <memory>0</memory>\n" - " <currentMemory>0</currentMemory>\n" + " <memory unit='KiB'>0</memory>\n" + " <currentMemory unit='KiB'>0</currentMemory>\n" " <vcpu>0</vcpu>\n" " <os>\n" " <type>exe</type>\n" diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 51f5aed..e13b87a 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -22,8 +22,9 @@ static int blankProblemElements(char *data) { if (virtTestClearLineRegex("<name>[[:alnum:]]+</name>", data) < 0 || virtTestClearLineRegex("<uuid>([[:alnum:]]|-)+</uuid>", data) < 0 || - virtTestClearLineRegex("<memory>[[:digit:]]+</memory>", data) < 0 || - virtTestClearLineRegex("<currentMemory>[[:digit:]]+</currentMemory>", data) < 0 || + virtTestClearLineRegex("<memory.*>[[:digit:]]+</memory>", data) < 0 || + virtTestClearLineRegex("<currentMemory.*>[[:digit:]]+</currentMemory>", + data) < 0 || virtTestClearLineRegex("<readonly/>", data) < 0 || virtTestClearLineRegex("<sharable/>", data) < 0) return -1; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-balloon-device-auto.xml b/tests/qemuxml2argvdata/qemuxml2argv-balloon-device-auto.xml index 9315621..fc909ae 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-balloon-device-auto.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-balloon-device-auto.xml @@ -1,8 +1,8 @@ <domain type='qemu'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory>219100</memory> - <currentMemory>219100</currentMemory> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> ... diff --git a/tests/xml2vmxdata/xml2vmx-svga.xml b/tests/xml2vmxdata/xml2vmx-svga.xml index b7db4c8..1148848 100644 --- a/tests/xml2vmxdata/xml2vmx-svga.xml +++ b/tests/xml2vmxdata/xml2vmx-svga.xml @@ -1,7 +1,7 @@ <domain type='vmware'> <name>minimal</name> <uuid>564d9bef-acd9-b4e0-c8f0-aea8b9103515</uuid> - <memory>4096</memory> + <memory unit='KiB'>4096</memory> <os> <type>hvm</type> </os> -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Make it obvious to 'dumpxml' readers what unit we are using, since our default of KiB for memory (1024) differs from qemu's default of MiB; while we use bytes for storage.
Tests were updated via:
$ find tests/*data tests/*out -name '*.xml' | \ xargs sed -i 's/<\(memory\|currentMemory\|hard_limit\|soft_limit\|min_guarantee\|swap_hard_limit\)>/<\1 unit='"'KiB'>/" $ find tests/*data tests/*out -name '*.xml' | \ xargs sed -i 's/<\(capacity\|allocation\|available\)>/<\1 unit='"'bytes'>/"
followed by a few fixes for the stragglers.
* docs/schemas/basictypes.rng (unit): Add 'bytes'. (scaledInteger): New define. * docs/schemas/storagevol.rng (sizing): Use it. * docs/schemas/storagepool.rng (sizing): Likewise. * docs/schemas/domaincommon.rng (memoryKBElement): New define; use for memory elements. * src/conf/storage_conf.c (virStoragePoolDefFormat) (virStorageVolDefFormat): Likewise. * src/conf/domain_conf.h (_virDomainDef): Document unit used internally. * src/conf/storage_conf.h (_virStoragePoolDef, _virStorageVolDef): Likewise. * tests/*data/*.xml: Update all tests. * tests/*out/*.xml: Likewise. * tests/define-dev-segfault: Likewise. * tests/openvzutilstest.c (testReadNetworkConf): Likewise. * tests/qemuargv2xmltest.c (blankProblemElements): Likewise. ---
corresponds to memory v1 1/3; v2: also output units for storage, use 'unit=' not 'units=', use common RNG
Portions of this patch elided to reduce mail size; see cover letter for git repo to see entire patch.
I didn't read the cover letter thoroughly enough to notice the repo, at first :(
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 4f16fa7..a50349c 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -140,8 +140,16 @@
<define name='unit'> <data type='string'> -<param name='pattern'>[kKmMgGtTpPeE]</param> +<param name='pattern'>(bytes)|[kKmMgGtTpPeE]</param>
Looking at this again. Don't you want to be able to specify the unit as "KiB" or just only as "k"?
</data> </define> +<define name='scaledInteger'> +<optional> +<attribute name='unit'> +<ref name='unit'/> +</attribute> +</optional> +<ref name='unsignedLong'/> +</define>
</grammar>
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f9654f1..331d923 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11638,8 +11638,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, xmlIndentTreeOutput = oldIndentTreeOutput; }
- virBufferAsprintf(buf, "<memory>%lu</memory>\n", def->mem.max_balloon); - virBufferAsprintf(buf, "<currentMemory>%lu</currentMemory>\n", + virBufferAsprintf(buf, "<memory unit='KiB'>%lu</memory>\n", + def->mem.max_balloon); + virBufferAsprintf(buf, "<currentMemory unit='KiB'>%lu</currentMemory>\n", def->mem.cur_balloon);
I'm not quite sure about this. When we print the unit and somebody tries to use the xml as a template for a new machine or simply to change the memory allocation for the domain using virsh edit, he might be tempted to change the unit to mibibytes and expect the amount of memory to be used. Instead of that he will get that amount in kilobytes and no warning about this. We probably should parse the unit and at least warn the user about this happening. With this patch applied to current upstream you will need to: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml index 65cd55d..b6bf1d4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml @@ -1,8 +1,8 @@ <domain type='qemu'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory>219136</memory> - <currentMemory>219136</currentMemory> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> to pass the tests. I'm not comfortable ACKing this one :(. I'd like to have some feedback from others on printing units without parsing them. Otherwise looks fine and passes the tests with that patch applied. Peter

On 03/06/2012 08:14 AM, Peter Krempa wrote:
On 03/06/2012 01:34 AM, Eric Blake wrote:
Make it obvious to 'dumpxml' readers what unit we are using, since our default of KiB for memory (1024) differs from qemu's default of MiB; while we use bytes for storage.
Tests were updated via:
+++ b/docs/schemas/basictypes.rng @@ -140,8 +140,16 @@
<define name='unit'> <data type='string'> -<param name='pattern'>[kKmMgGtTpPeE]</param> +<param name='pattern'>(bytes)|[kKmMgGtTpPeE]</param>
Looking at this again. Don't you want to be able to specify the unit as "KiB" or just only as "k"?
Yes, and that gets fixed later in the series (see 6/15 and 10/15). This was the minimum change to get 'make check' to pass at this stage of the series before I start adding new parsing abilities in later patches. Also, note that domaincommon.rng _isn't_ using <ref name='unit'/> until patch 10/15; in this patch, domaincommon.rng is using an open-coded RNG that accepts _only_ <attribute name='unit'><value>KiB</value></attribute>.
- virBufferAsprintf(buf, "<currentMemory>%lu</currentMemory>\n", + virBufferAsprintf(buf, "<memory unit='KiB'>%lu</memory>\n", + def->mem.max_balloon); + virBufferAsprintf(buf, "<currentMemory unit='KiB'>%lu</currentMemory>\n", def->mem.cur_balloon);
I'm not quite sure about this. When we print the unit and somebody tries to use the xml as a template for a new machine or simply to change the memory allocation for the domain using virsh edit, he might be tempted to change the unit to mibibytes and expect the amount of memory to be used. Instead of that he will get that amount in kilobytes and no warning about this. We probably should parse the unit and at least warn the user about this happening.
Yes, that happens in 10/15.
With this patch applied to current upstream you will need to: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml index 65cd55d..b6bf1d4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml @@ -1,8 +1,8 @@ <domain type='qemu'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory>219136</memory> - <currentMemory>219136</currentMemory> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory>
Ah, new tests added in the meantime. Yes, when I rebase, I'll make sure things still work.
to pass the tests.
I'm not comfortable ACKing this one :(. I'd like to have some feedback from others on printing units without parsing them. Otherwise looks fine and passes the tests with that patch applied.
If anything, I can modify the commit message to mention that a later patch will parse units. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/06/2012 04:14 PM, Peter Krempa wrote:
On 03/06/2012 01:34 AM, Eric Blake wrote:
Make it obvious to 'dumpxml' readers what unit we are using, since our default of KiB for memory (1024) differs from qemu's default of MiB; while we use bytes for storage.
Tests were updated via:
$ find tests/*data tests/*out -name '*.xml' | \ xargs sed -i 's/<\(memory\|currentMemory\|hard_limit\|soft_limit\|min_guarantee\|swap_hard_limit\)>/<\1 unit='"'KiB'>/" $ find tests/*data tests/*out -name '*.xml' | \ xargs sed -i 's/<\(capacity\|allocation\|available\)>/<\1 unit='"'bytes'>/"
followed by a few fixes for the stragglers.
* docs/schemas/basictypes.rng (unit): Add 'bytes'. (scaledInteger): New define. * docs/schemas/storagevol.rng (sizing): Use it. * docs/schemas/storagepool.rng (sizing): Likewise. * docs/schemas/domaincommon.rng (memoryKBElement): New define; use for memory elements. * src/conf/storage_conf.c (virStoragePoolDefFormat) (virStorageVolDefFormat): Likewise. * src/conf/domain_conf.h (_virDomainDef): Document unit used internally. * src/conf/storage_conf.h (_virStoragePoolDef, _virStorageVolDef): Likewise. * tests/*data/*.xml: Update all tests. * tests/*out/*.xml: Likewise. * tests/define-dev-segfault: Likewise. * tests/openvzutilstest.c (testReadNetworkConf): Likewise. * tests/qemuargv2xmltest.c (blankProblemElements): Likewise. ---
corresponds to memory v1 1/3; v2: also output units for storage, use 'unit=' not 'units=', use common RNG
Portions of this patch elided to reduce mail size; see cover letter for git repo to see entire patch.
I didn't read the cover letter thoroughly enough to notice the repo, at first :(
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 4f16fa7..a50349c 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -140,8 +140,16 @@
<define name='unit'> <data type='string'> -<param name='pattern'>[kKmMgGtTpPeE]</param> +<param name='pattern'>(bytes)|[kKmMgGtTpPeE]</param>
Looking at this again. Don't you want to be able to specify the unit as "KiB" or just only as "k"?
</data> </define> +<define name='scaledInteger'> +<optional> +<attribute name='unit'> +<ref name='unit'/> +</attribute> +</optional> +<ref name='unsignedLong'/> +</define>
</grammar>
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f9654f1..331d923 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11638,8 +11638,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, xmlIndentTreeOutput = oldIndentTreeOutput; }
- virBufferAsprintf(buf, "<memory>%lu</memory>\n", def->mem.max_balloon); - virBufferAsprintf(buf, "<currentMemory>%lu</currentMemory>\n", + virBufferAsprintf(buf, "<memory unit='KiB'>%lu</memory>\n", + def->mem.max_balloon); + virBufferAsprintf(buf, "<currentMemory unit='KiB'>%lu</currentMemory>\n", def->mem.cur_balloon);
I'm not quite sure about this. When we print the unit and somebody tries to use the xml as a template for a new machine or simply to change the memory allocation for the domain using virsh edit, he might be tempted to change the unit to mibibytes and expect the amount of memory to be used. Instead of that he will get that amount in kilobytes and no warning about this. We probably should parse the unit and at least warn the user about this happening.
With this patch applied to current upstream you will need to: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml index 65cd55d..b6bf1d4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml @@ -1,8 +1,8 @@ <domain type='qemu'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory>219136</memory> - <currentMemory>219136</currentMemory> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type>
to pass the tests.
I'm not comfortable ACKing this one :(. I'd like to have some feedback from others on printing units without parsing them. Otherwise looks fine and passes the tests with that patch applied.
Peter
After reviewing 10/15 I'm now comfortable with ACKing this one if 10/15 gets in :) Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Disk manufacturers are fond of quoting sizes in powers of 10, rather than powers of 2 (after all, 2.1 GB sounds larger than 2.0 GiB, even though the exact opposite is true). So, we might as well follow coreutils lead in supporting three types of suffix: single letter ${u} (which we already had) and ${u}iB for the power of 2, and ${u}B for power of 10. Additionally, it is impossible to create a file with more than 2**63 bytes, since off_t is signed (if you have enough storage to even create one 8EiB file, I'm jealous). This now reports failure up front rather than down the road when the kernel finally refuses an impossible size. * docs/schemas/basictypes.rng (unit): Add suffixes. * src/conf/storage_conf.c (virStorageSize): Use new function. * docs/formatstorage.html.in: Document it. --- v2: new docs/formatstorage.html.in | 21 +++++--- docs/schemas/basictypes.rng | 2 +- src/conf/storage_conf.c | 61 +++--------------------- tests/storagevolxml2xmlin/vol-file-backing.xml | 4 +- tests/storagevolxml2xmlin/vol-file.xml | 4 +- 5 files changed, 26 insertions(+), 66 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 93d8ab2..b0acaf3 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -236,13 +236,20 @@ <br/> By default this is specified in bytes, but an optional <code>unit</code> can be specified to adjust the passed value. - Values can be: 'K' (kibibytes, 2<sup>10</sup> or 1024), 'M' - (mebibytes, 2<sup>20</sup> or 1,048,576), 'G' (gibibytes, - 2<sup>30</sup> or 1,073,741,824), 'T' (tebibytes, - 2<sup>40</sup> or 1,099,511,627,776), 'P' (pebibytes, - 2<sup>50</sup> or 1,125,899,906,842,624), or 'E' (exbibytes, - 2<sup>60</sup> or 1,152,921,504,606,846,976). - <span class="since">Since 0.4.1</span></dd> + Values can be: 'B' or 'bytes' for bytes, 'KB' (kilobytes, + 10<sup>3</sup> or 1000), 'K' or 'KiB' (kibibytes, + 2<sup>10</sup> or 1024), 'MB' (megabytes, 10<sup>6</sup> or + 1,000,000), 'M' or 'MiB' (mebibytes, 2<sup>20</sup> or + 1,048,576), 'GB' (gigabytes, 10<sup>9</sup> or 1,000,000,000), + 'G' or 'GiB' (gibibytes, 2<sup>30</sup> or 1,073,741,824), + 'TB' (terabytes, 10<sup>12</sup> or 1,000,000,000,000), 'T' or + 'TiB' (tebibytes, 2<sup>40</sup> or 1,099,511,627,776), 'PB' + (petabytes, 10<sup>15</sup> or 1,000,000,000,000,000), 'P' or + 'PiB' (pebibytes, 2<sup>50</sup> or 1,125,899,906,842,624), + 'EB' (exabytes, 10<sup>18</sup> or 1,000,000,000,000,000,000), + or 'E' or 'EiB' (exbibytes, 2<sup>60</sup> or + 1,152,921,504,606,846,976). <span class="since">Since 0.4.1, + multi-character <code>unit</code> since 0.9.11</span></dd> <dt><code>capacity</code></dt> <dd>Providing the logical capacity for the volume. This value is in bytes by default, but a <code>unit</code> attribute can be diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index a50349c..cc0bc12 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -140,7 +140,7 @@ <define name='unit'> <data type='string'> - <param name='pattern'>(bytes)|[kKmMgGtTpPeE]</param> + <param name='pattern'>([bB]([yY][tT][eE][sS]?)?)|([kKmMgGtTpPeE]([iI]?[bB])?)</param> </data> </define> <define name='scaledInteger'> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 07f3f5b..d1558ee 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -944,64 +944,17 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) { static int virStorageSize(const char *unit, const char *val, - unsigned long long *ret) { - unsigned long long mult; - char *end; - - if (!unit) { - mult = 1; - } else { - switch (unit[0]) { - case 'k': - case 'K': - mult = 1024ull; - break; - - case 'm': - case 'M': - mult = 1024ull * 1024ull; - break; - - case 'g': - case 'G': - mult = 1024ull * 1024ull * 1024ull; - break; - - case 't': - case 'T': - mult = 1024ull * 1024ull * 1024ull * 1024ull; - break; - - case 'p': - case 'P': - mult = 1024ull * 1024ull * 1024ull * 1024ull * 1024ull; - break; - - case 'e': - case 'E': - mult = 1024ull * 1024ull * 1024ull * 1024ull * 1024ull * - 1024ull; - break; - - default: - virStorageReportError(VIR_ERR_XML_ERROR, - _("unknown size units '%s'"), unit); - return -1; - } - } - - if (virStrToLong_ull (val, &end, 10, ret) < 0) { + unsigned long long *ret) +{ + if (virStrToLong_ull(val, NULL, 10, ret) < 0) { virStorageReportError(VIR_ERR_XML_ERROR, "%s", _("malformed capacity element")); return -1; } - if (*ret > (ULLONG_MAX / mult)) { - virStorageReportError(VIR_ERR_XML_ERROR, - "%s", _("capacity element value too large")); - return -1; - } - - *ret *= mult; + /* off_t is signed, so you cannot create a file larger than 2^^63 + * bytes in the first place. */ + if (virScaleInteger(ret, unit, 1, LLONG_MAX) < 0) + return -1; return 0; } diff --git a/tests/storagevolxml2xmlin/vol-file-backing.xml b/tests/storagevolxml2xmlin/vol-file-backing.xml index c1a5837..d23349e 100644 --- a/tests/storagevolxml2xmlin/vol-file-backing.xml +++ b/tests/storagevolxml2xmlin/vol-file-backing.xml @@ -1,8 +1,8 @@ <volume> <name>sparse.img</name> <source/> - <capacity>10000000000</capacity> - <allocation>0</allocation> + <capacity unit='GB'>10</capacity> + <allocation unit='MiB'>0</allocation> <target> <path>/var/lib/libvirt/images/sparse.img</path> <permissions> diff --git a/tests/storagevolxml2xmlin/vol-file.xml b/tests/storagevolxml2xmlin/vol-file.xml index d7de0aa..fdca510 100644 --- a/tests/storagevolxml2xmlin/vol-file.xml +++ b/tests/storagevolxml2xmlin/vol-file.xml @@ -1,8 +1,8 @@ <volume> <name>sparse.img</name> <source/> - <capacity unit="T">1</capacity> - <allocation>0</allocation> + <capacity unit="TiB">1</capacity> + <allocation unit="bytes">0</allocation> <target> <path>/var/lib/libvirt/images/sparse.img</path> <permissions> -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Disk manufacturers are fond of quoting sizes in powers of 10, rather than powers of 2 (after all, 2.1 GB sounds larger than 2.0 GiB, even though the exact opposite is true). So, we might as well follow coreutils lead in supporting three types of suffix: single letter ${u} (which we already had) and ${u}iB for the power of 2, and ${u}B for power of 10.
Additionally, it is impossible to create a file with more than 2**63 bytes, since off_t is signed (if you have enough storage to even create one 8EiB file, I'm jealous). This now reports failure up front rather than down the road when the kernel finally refuses an impossible size.
* docs/schemas/basictypes.rng (unit): Add suffixes. * src/conf/storage_conf.c (virStorageSize): Use new function. * docs/formatstorage.html.in: Document it. ---
v2: new
docs/formatstorage.html.in | 21 +++++--- docs/schemas/basictypes.rng | 2 +- src/conf/storage_conf.c | 61 +++--------------------- tests/storagevolxml2xmlin/vol-file-backing.xml | 4 +- tests/storagevolxml2xmlin/vol-file.xml | 4 +- 5 files changed, 26 insertions(+), 66 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 93d8ab2..b0acaf3 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -236,13 +236,20 @@ <br/> By default this is specified in bytes, but an optional <code>unit</code> can be specified to adjust the passed value. - Values can be: 'K' (kibibytes, 2<sup>10</sup> or 1024), 'M' - (mebibytes, 2<sup>20</sup> or 1,048,576), 'G' (gibibytes, - 2<sup>30</sup> or 1,073,741,824), 'T' (tebibytes, - 2<sup>40</sup> or 1,099,511,627,776), 'P' (pebibytes, - 2<sup>50</sup> or 1,125,899,906,842,624), or 'E' (exbibytes, - 2<sup>60</sup> or 1,152,921,504,606,846,976). -<span class="since">Since 0.4.1</span></dd> + Values can be: 'B' or 'bytes' for bytes, 'KB' (kilobytes, + 10<sup>3</sup> or 1000), 'K' or 'KiB' (kibibytes, + 2<sup>10</sup> or 1024), 'MB' (megabytes, 10<sup>6</sup> or + 1,000,000), 'M' or 'MiB' (mebibytes, 2<sup>20</sup> or + 1,048,576), 'GB' (gigabytes, 10<sup>9</sup> or 1,000,000,000), + 'G' or 'GiB' (gibibytes, 2<sup>30</sup> or 1,073,741,824),
As Christophe pointed out on 01/15, it would probably be better to state that the numbers are in bytes.
+ 'TB' (terabytes, 10<sup>12</sup> or 1,000,000,000,000), 'T' or + 'TiB' (tebibytes, 2<sup>40</sup> or 1,099,511,627,776), 'PB' + (petabytes, 10<sup>15</sup> or 1,000,000,000,000,000), 'P' or + 'PiB' (pebibytes, 2<sup>50</sup> or 1,125,899,906,842,624), + 'EB' (exabytes, 10<sup>18</sup> or 1,000,000,000,000,000,000), + or 'E' or 'EiB' (exbibytes, 2<sup>60</sup> or + 1,152,921,504,606,846,976).<span class="since">Since 0.4.1, + multi-character<code>unit</code> since 0.9.11</span></dd> <dt><code>capacity</code></dt> <dd>Providing the logical capacity for the volume. This value is in bytes by default, but a<code>unit</code> attribute can be diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index a50349c..cc0bc12 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -140,7 +140,7 @@
<define name='unit'> <data type='string'> -<param name='pattern'>(bytes)|[kKmMgGtTpPeE]</param> +<param name='pattern'>([bB]([yY][tT][eE][sS]?)?)|([kKmMgGtTpPeE]([iI]?[bB])?)</param>
This makes the comment from the last patch obsolete.
</data> </define> <define name='scaledInteger'>
ACK, Peter

The test domain allows <memory>0</memory>, but the RNG was stating that memory had to be at least 4096000 bytes. Hypervisors should enforce their own limits, rather than complicating the RNG. Meanwhile, some copy and paste had introduced some fishy constructs in various unit tests. * docs/schemas/domaincommon.rng (memoryKB, memoryKBElement): Drop limit that isn't enforced in code. * src/conf/domain_conf.c (virDomainDefParseXML): Require current <= maximum. * tests/qemuxml2argvdata/*.xml: Fix offenders. --- rework of memory v1 2/3 v2: fix bogus comment regarding 4000KiB limit docs/schemas/domaincommon.rng | 7 +------ src/conf/domain_conf.c | 7 +++++++ .../qemuxml2argv-smartcard-controller.xml | 2 +- .../qemuxml2argv-smartcard-host-certificates.xml | 2 +- .../qemuxml2argv-smartcard-host.xml | 2 +- ...qemuxml2argv-smartcard-passthrough-spicevmc.xml | 2 +- .../qemuxml2argv-smartcard-passthrough-tcp.xml | 2 +- .../qemuxml2argv-usb-controller.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml | 2 +- .../qemuxml2argv-usb-ich9-companion.xml | 2 +- .../qemuxml2argv-usb-ich9-ehci-addr.xml | 2 +- .../qemuxml2argv-usb-piix3-controller.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml | 2 +- 15 files changed, 21 insertions(+), 19 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c8b403e..99b3861 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3027,10 +3027,6 @@ <!-- Type library - - A domain name should be made of ascii, numbers, _-+ and is non-empty - memoryKB request at least 4Mbytes though Xen will grow bigger if too low - weight currently is in range [100, 1000] --> <define name="cpuset"> <data type="string"> @@ -3072,6 +3068,7 @@ <param name="minInclusive">-1</param> </data> </define> + <!-- weight currently is in range [100, 1000] --> <define name="weight"> <data type="unsignedInt"> <param name="pattern">[0-9]+</param> @@ -3083,7 +3080,6 @@ <define name="memoryKB"> <data type="unsignedInt"> <param name="pattern">[0-9]+</param> - <param name="minInclusive">4000</param> </data> </define> <!-- Memory as an element, with optional unit attribute --> @@ -3095,7 +3091,6 @@ </optional> <data type='unsignedInt'> <param name='pattern'>[0-9]+</param> - <param name='minInclusive'>4000</param> </data> </define> <define name="domainName"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 331d923..41d0eca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7303,6 +7303,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, &def->mem.cur_balloon) < 0) def->mem.cur_balloon = def->mem.max_balloon; + if (def->mem.cur_balloon > def->mem.max_balloon) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("current memory '%luk' exceeds maximum '%luk'"), + def->mem.cur_balloon, def->mem.max_balloon); + goto error; + } + node = virXPathNode("./memoryBacking/hugepages", ctxt); if (node) def->mem.hugepage_backed = 1; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.xml index 1914c6b..3256010 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-controller.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.xml index 2f58068..d91afcc 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.xml index b0415b3..7069279 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-host.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml index 33e2ee4..db42a11 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.xml index 92fb31d..05a93c3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-tcp.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-controller.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller.xml index 45d81b6..66657cf 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-controller.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml index 14490e5..c013014 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml index b7b47cd..d12f67e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml index 3237ef9..aa6c092 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.xml index d233544..58dbfb1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml index 0cedf87..a6eebf5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml index b0e8b45..fd23ec8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml index 572cb2b..b8a2ccf 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml @@ -2,7 +2,7 @@ <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219200</currentMemory> + <currentMemory unit='KiB'>219136</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc'>hvm</type> -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
The test domain allows<memory>0</memory>, but the RNG was stating that memory had to be at least 4096000 bytes. Hypervisors should enforce their own limits, rather than complicating the RNG.
Meanwhile, some copy and paste had introduced some fishy constructs in various unit tests.
* docs/schemas/domaincommon.rng (memoryKB, memoryKBElement): Drop limit that isn't enforced in code. * src/conf/domain_conf.c (virDomainDefParseXML): Require current <= maximum. * tests/qemuxml2argvdata/*.xml: Fix offenders. ---
ACK, Peter

On 64-bit platforms, unsigned long and unsigned long long are identical, so we don't have to worry about overflow checks. On 32-bit platforms, anywhere we narrow unsigned long long back to unsigned long, we have to worry about overflow; it's easier to do this in one place by having most of the code use the same or wider types, and only doing the narrowing at the last minute. Therefore, the memory set commands remain unsigned long, and the memory get command now centralizes the overflow check into libvirt.c, so that drivers don't have to repeat the work. * src/driver.h (virDrvDomainGetMaxMemory): Use long long. * src/libvirt.c (virDomainGetMaxMemory): Raise overflow. * src/test/test_driver.c (testGetMaxMemory): Fix driver. * src/rpc/gendispatch.pl (name_to_ProcName): Likewise. * src/xen/xen_hypervisor.c (xenHypervisorGetMaxMemory): Likewise. * src/xen/xen_driver.c (xenUnifiedDomainGetMaxMemory): Likewise. * src/xen/xend_internal.c (xenDaemonDomainGetMaxMemory): Likewise. * src/xen/xend_internal.h (xenDaemonDomainGetMaxMemory): Likewise. * src/xen/xm_internal.c (xenXMDomainGetMaxMemory): Likewise. * src/xen/xm_internal.h (xenXMDomainGetMaxMemory): Likewise. * src/xen/xs_internal.c (xenStoreDomainGetMaxMemory): Likewise. * src/xen/xs_internal.h (xenStoreDomainGetMaxMemory): Likewise. * src/xenapi/xenapi_driver.c (xenapiDomainGetMaxMemory): Likewise. * src/esx/esx_driver.c (esxDomainGetMaxMemory): Likewise. * src/libxl/libxl_driver.c (libxlDomainGetMaxMemory): Likewise. * src/qemu/qemu_driver.c (qemudDomainGetMaxMemory): Likewise. * src/lxc/lxc_driver.c (lxcDomainGetMaxMemory): Likewise. * src/uml/uml_driver.c (umlDomainGetMaxMemory): Likewise. --- v2: new src/driver.h | 2 +- src/esx/esx_driver.c | 4 ++-- src/libvirt.c | 7 ++++++- src/libxl/libxl_driver.c | 8 ++++---- src/lxc/lxc_driver.c | 6 ++++-- src/qemu/qemu_driver.c | 8 +++++--- src/rpc/gendispatch.pl | 1 - src/test/test_driver.c | 4 ++-- src/uml/uml_driver.c | 6 ++++-- src/xen/xen_driver.c | 6 +++--- src/xen/xen_hypervisor.c | 4 ++-- src/xen/xend_internal.c | 12 ++++++------ src/xen/xend_internal.h | 4 ++-- src/xen/xm_internal.c | 6 +++--- src/xen/xm_internal.h | 4 ++-- src/xen/xs_internal.c | 10 +++++----- src/xen/xs_internal.h | 4 ++-- src/xenapi/xenapi_driver.c | 6 +++--- 18 files changed, 56 insertions(+), 46 deletions(-) diff --git a/src/driver.h b/src/driver.h index 212d2f5..03d249b 100644 --- a/src/driver.h +++ b/src/driver.h @@ -142,7 +142,7 @@ typedef int unsigned int flags); typedef char * (*virDrvDomainGetOSType) (virDomainPtr domain); -typedef unsigned long +typedef unsigned long long (*virDrvDomainGetMaxMemory) (virDomainPtr domain); typedef int (*virDrvDomainSetMaxMemory) (virDomainPtr domain, diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index b6b22f8..6943534 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2,7 +2,7 @@ /* * esx_driver.c: core driver functions for managing VMware ESX hosts * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2009-2011 Matthias Bolte <matthias.bolte@googlemail.com> * Copyright (C) 2009 Maximilian Wilhelm <max@rfc2324.org> * @@ -2073,7 +2073,7 @@ esxDomainGetOSType(virDomainPtr domain ATTRIBUTE_UNUSED) -static unsigned long +static unsigned long long esxDomainGetMaxMemory(virDomainPtr domain) { esxPrivate *priv = domain->conn->privateData; diff --git a/src/libvirt.c b/src/libvirt.c index de23db8..73fb8de 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3602,10 +3602,15 @@ virDomainGetMaxMemory(virDomainPtr domain) conn = domain->conn; if (conn->driver->domainGetMaxMemory) { - unsigned long ret; + unsigned long long ret; ret = conn->driver->domainGetMaxMemory (domain); if (ret == 0) goto error; + if ((unsigned long) ret != ret) { + virLibDomainError(VIR_ERR_OVERFLOW, _("result too large: %llu"), + ret); + goto error; + } return ret; } diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d5fa64a..a7bb751 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1,7 +1,7 @@ /*---------------------------------------------------------------------------*/ -/* Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany. +/* Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany. * Copyright (C) 2011 Univention GmbH. - * Copyright (C) 2006-2011 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -1592,12 +1592,12 @@ cleanup: return type; } -static unsigned long +static unsigned long long libxlDomainGetMaxMemory(virDomainPtr dom) { libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; - unsigned long ret = 0; + unsigned long long ret = 0; libxlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index d9cbd9e..1a0e458 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -670,10 +670,12 @@ cleanup: } /* Returns max memory in kb, 0 if error */ -static unsigned long lxcDomainGetMaxMemory(virDomainPtr dom) { +static unsigned long long +lxcDomainGetMaxMemory(virDomainPtr dom) +{ lxc_driver_t *driver = dom->conn->privateData; virDomainObjPtr vm; - unsigned long ret = 0; + unsigned long long ret = 0; lxcDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1bdd2f0..aee95e4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1877,10 +1877,12 @@ cleanup: } /* Returns max memory in kb, 0 if error */ -static unsigned long qemudDomainGetMaxMemory(virDomainPtr dom) { +static unsigned long long +qemuDomainGetMaxMemory(virDomainPtr dom) +{ struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - unsigned long ret = 0; + unsigned long long ret = 0; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -12264,7 +12266,7 @@ static virDriver qemuDriver = { .domainDestroy = qemuDomainDestroy, /* 0.2.0 */ .domainDestroyFlags = qemuDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = qemudDomainGetOSType, /* 0.2.2 */ - .domainGetMaxMemory = qemudDomainGetMaxMemory, /* 0.4.2 */ + .domainGetMaxMemory = qemuDomainGetMaxMemory, /* 0.4.2 */ .domainSetMaxMemory = qemudDomainSetMaxMemory, /* 0.4.2 */ .domainSetMemory = qemudDomainSetMemory, /* 0.4.2 */ .domainSetMemoryFlags = qemudDomainSetMemoryFlags, /* 0.9.0 */ diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 3f37d58..ef369b9 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -201,7 +201,6 @@ close(PROTOCOL); # this list is fixed. new procedures and public APIs have to map [unsigned] # hyper to [unsigned] long long my $long_legacy = { - DomainGetMaxMemory => { ret => { memory => 1 } }, DomainGetInfo => { ret => { maxMem => 1, memory => 1 } }, DomainMigrate => { arg => { flags => 1, resource => 1 } }, DomainMigrate2 => { arg => { flags => 1, resource => 1 } }, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 39fd63d..f03b2fd 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2021,10 +2021,10 @@ static char *testGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED) { return ret; } -static unsigned long testGetMaxMemory(virDomainPtr domain) { +static unsigned long long testGetMaxMemory(virDomainPtr domain) { testConnPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; - unsigned long ret = 0; + unsigned long long ret = 0; testDriverLock(privconn); privdom = virDomainFindByName(&privconn->domains, diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 378dffc..16818cd 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1625,10 +1625,12 @@ cleanup: } /* Returns max memory in kb, 0 if error */ -static unsigned long umlDomainGetMaxMemory(virDomainPtr dom) { +static unsigned long long +umlDomainGetMaxMemory(virDomainPtr dom) +{ struct uml_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - unsigned long ret = 0; + unsigned long long ret = 0; umlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 19ce7da..0238ed7 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1,7 +1,7 @@ /* * xen_driver.c: Unified Xen driver. * - * Copyright (C) 2007-2011 Red Hat, Inc. + * Copyright (C) 2007-2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -937,12 +937,12 @@ xenUnifiedDomainGetOSType (virDomainPtr dom) return NULL; } -static unsigned long +static unsigned long long xenUnifiedDomainGetMaxMemory (virDomainPtr dom) { GET_PRIVATE(dom->conn); int i; - unsigned long ret; + unsigned long long ret; for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) if (priv->opened[i] && drivers[i]->xenDomainGetMaxMemory) { diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index b5b2328..4401b68 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -855,7 +855,7 @@ typedef struct xen_op_v2_dom xen_op_v2_dom; # error "unsupported platform" #endif -static unsigned long xenHypervisorGetMaxMemory(virDomainPtr domain); +static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain); struct xenUnifiedDriver xenHypervisorDriver = { .xenClose = xenHypervisorClose, @@ -3157,7 +3157,7 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id) * * Returns the memory size in kilobytes or 0 in case of error. */ -static unsigned long ATTRIBUTE_NONNULL (1) +static unsigned long long ATTRIBUTE_NONNULL (1) xenHypervisorGetMaxMemory(virDomainPtr domain) { xenUnifiedPrivatePtr priv; diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 83bfac0..691ce17 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1,7 +1,7 @@ /* * xend_internal.c: access to Xen though the Xen Daemon interface * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori <aliguori@us.ibm.com> * * This file is subject to the terms and conditions of the GNU Lesser General @@ -1638,29 +1638,29 @@ xenDaemonDomainRestore(virConnectPtr conn, const char *filename) * * Returns the memory size in kilobytes or 0 in case of error. */ -unsigned long +unsigned long long xenDaemonDomainGetMaxMemory(virDomainPtr domain) { - unsigned long ret = 0; + unsigned long long ret = 0; struct sexpr *root; xenUnifiedPrivatePtr priv; if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return(-1); + return(0); } priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) - return(-1); + return(0); /* can we ask for a subset ? worth it ? */ root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name); if (root == NULL) return(0); - ret = (unsigned long) sexpr_u64(root, "domain/memory") << 10; + ret = sexpr_u64(root, "domain/memory") << 10; sexpr_free(root); return(ret); diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 3f0b63d..5942788 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -1,7 +1,7 @@ /* * xend_internal.h * - * Copyright (C) 2006-2008, 2010-2011 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010-2012 Red Hat, Inc. * Copyright (C) 2005,2006 * * Anthony Liguori <aliguori@us.ibm.com> @@ -119,7 +119,7 @@ int xenDaemonDomainGetState(virDomainPtr domain, unsigned int flags); char *xenDaemonDomainGetXMLDesc(virDomainPtr domain, unsigned int flags, const char *cpus); -unsigned long xenDaemonDomainGetMaxMemory(virDomainPtr domain); +unsigned long long xenDaemonDomainGetMaxMemory(virDomainPtr domain); char **xenDaemonListDomainsOld(virConnectPtr xend); virDomainPtr xenDaemonDomainDefineXML(virConnectPtr xend, const char *sexpr); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 5acac8b..e966e63 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1,7 +1,7 @@ /* * xm_internal.h: helper routines for dealing with inactive domains * - * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -646,11 +646,11 @@ cleanup: /* * Get max memory limit from config */ -unsigned long xenXMDomainGetMaxMemory(virDomainPtr domain) { +unsigned long long xenXMDomainGetMaxMemory(virDomainPtr domain) { xenUnifiedPrivatePtr priv; const char *filename; xenXMConfCachePtr entry; - unsigned long ret = 0; + unsigned long long ret = 0; if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { xenXMError(VIR_ERR_INVALID_ARG, __FUNCTION__); diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h index 89af23e..3e1f886 100644 --- a/src/xen/xm_internal.h +++ b/src/xen/xm_internal.h @@ -1,7 +1,7 @@ /* * xm_internal.h: helper routines for dealing with inactive domains * - * Copyright (C) 2006-2007, 2010-2011 Red Hat, Inc. + * Copyright (C) 2006-2007, 2010-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -48,7 +48,7 @@ int xenXMDomainGetState(virDomainPtr domain, char *xenXMDomainGetXMLDesc(virDomainPtr domain, unsigned int flags); int xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory); int xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory); -unsigned long xenXMDomainGetMaxMemory(virDomainPtr domain); +unsigned long long xenXMDomainGetMaxMemory(virDomainPtr domain); int xenXMDomainSetVcpus(virDomainPtr domain, unsigned int vcpus); int xenXMDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, unsigned int flags); diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 86e5519..10811dd 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -1,7 +1,7 @@ /* * xs_internal.c: access to Xen Store * - * Copyright (C) 2006, 2009-2011 Red Hat, Inc. + * Copyright (C) 2006, 2009-2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -495,23 +495,23 @@ xenStoreDomainSetMemory(virDomainPtr domain, unsigned long memory) * * Returns the memory size in kilobytes or 0 in case of error. */ -unsigned long +unsigned long long xenStoreDomainGetMaxMemory(virDomainPtr domain) { char *tmp; - unsigned long ret = 0; + unsigned long long ret = 0; xenUnifiedPrivatePtr priv; if (!VIR_IS_CONNECTED_DOMAIN(domain)) return (ret); if (domain->id == -1) - return(-1); + return(0); priv = domain->conn->privateData; xenUnifiedLock(priv); tmp = virDomainDoStoreQuery(domain->conn, domain->id, "memory/target"); if (tmp != NULL) { - ret = (unsigned long) atol(tmp); + ret = atol(tmp); VIR_FREE(tmp); } xenUnifiedUnlock(priv); diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h index f7e487b..1ada0f3 100644 --- a/src/xen/xs_internal.h +++ b/src/xen/xs_internal.h @@ -1,7 +1,7 @@ /* * xs_internal.h: internal API for access to XenStore * - * Copyright (C) 2006, 2010-2011 Red Hat, Inc. + * Copyright (C) 2006, 2010-2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -36,7 +36,7 @@ virDomainPtr xenStoreLookupByName(virConnectPtr conn, unsigned long xenStoreGetMaxMemory (virDomainPtr domain); int xenStoreDomainSetMemory (virDomainPtr domain, unsigned long memory); -unsigned long xenStoreDomainGetMaxMemory(virDomainPtr domain); +unsigned long long xenStoreDomainGetMaxMemory(virDomainPtr domain); int xenStoreDomainShutdown (virDomainPtr domain); int xenStoreDomainReboot (virDomainPtr domain, unsigned int flags); diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 94644ae..298db12 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1,6 +1,6 @@ /* * xenapi_driver.c: Xen API driver. - * Copyright (C) 2011 Red Hat, Inc. + * Copyright (C) 2011-2012 Red Hat, Inc. * Copyright (C) 2009, 2010 Citrix Ltd. * * This library is free software; you can redistribute it and/or @@ -955,7 +955,7 @@ xenapiDomainGetOSType (virDomainPtr dom) * Returns maximum static memory for VM on success * or 0 in case of error */ -static unsigned long +static unsigned long long xenapiDomainGetMaxMemory (virDomainPtr dom) { int64_t mem_static_max = 0; @@ -972,7 +972,7 @@ xenapiDomainGetMaxMemory (virDomainPtr dom) vm = vms->contents[0]; xen_vm_get_memory_static_max(session, &mem_static_max, vm); xen_vm_set_free(vms); - return (unsigned long)(mem_static_max / 1024); + return (mem_static_max / 1024); } else { if (vms) xen_vm_set_free(vms); xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL); -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
On 64-bit platforms, unsigned long and unsigned long long are identical, so we don't have to worry about overflow checks. On 32-bit platforms, anywhere we narrow unsigned long long back to unsigned long, we have to worry about overflow; it's easier to do this in one place by having most of the code use the same or wider types, and only doing the narrowing at the last minute. Therefore, the memory set commands remain unsigned long, and the memory get command now centralizes the overflow check into libvirt.c, so that drivers don't have to repeat the work.
* src/driver.h (virDrvDomainGetMaxMemory): Use long long. * src/libvirt.c (virDomainGetMaxMemory): Raise overflow. * src/test/test_driver.c (testGetMaxMemory): Fix driver. * src/rpc/gendispatch.pl (name_to_ProcName): Likewise. * src/xen/xen_hypervisor.c (xenHypervisorGetMaxMemory): Likewise. * src/xen/xen_driver.c (xenUnifiedDomainGetMaxMemory): Likewise. * src/xen/xend_internal.c (xenDaemonDomainGetMaxMemory): Likewise. * src/xen/xend_internal.h (xenDaemonDomainGetMaxMemory): Likewise. * src/xen/xm_internal.c (xenXMDomainGetMaxMemory): Likewise. * src/xen/xm_internal.h (xenXMDomainGetMaxMemory): Likewise. * src/xen/xs_internal.c (xenStoreDomainGetMaxMemory): Likewise. * src/xen/xs_internal.h (xenStoreDomainGetMaxMemory): Likewise. * src/xenapi/xenapi_driver.c (xenapiDomainGetMaxMemory): Likewise. * src/esx/esx_driver.c (esxDomainGetMaxMemory): Likewise. * src/libxl/libxl_driver.c (libxlDomainGetMaxMemory): Likewise. * src/qemu/qemu_driver.c (qemudDomainGetMaxMemory): Likewise. * src/lxc/lxc_driver.c (lxcDomainGetMaxMemory): Likewise. * src/uml/uml_driver.c (umlDomainGetMaxMemory): Likewise. ---
@@ -495,23 +495,23 @@ xenStoreDomainSetMemory(virDomainPtr domain, unsigned long memory) * * Returns the memory size in kilobytes or 0 in case of error. */ -unsigned long +unsigned long long xenStoreDomainGetMaxMemory(virDomainPtr domain) { char *tmp; - unsigned long ret = 0; + unsigned long long ret = 0; xenUnifiedPrivatePtr priv;
if (!VIR_IS_CONNECTED_DOMAIN(domain)) return (ret); if (domain->id == -1) - return(-1); + return(0);
This was scary!
priv = domain->conn->privateData; xenUnifiedLock(priv); tmp = virDomainDoStoreQuery(domain->conn, domain->id, "memory/target"); if (tmp != NULL) { - ret = (unsigned long) atol(tmp); + ret = atol(tmp); VIR_FREE(tmp); } xenUnifiedUnlock(priv);
ACK, Peter

Using 'unsigned long' for memory values is risky on 32-bit platforms, as a PAE guest can have more than 4GiB memory. Our API is (unfortunately) locked at 'unsigned long' and a scale of 1024, but the rest of our system should consistently use 64-bit values, especially since the previous patch centralized overflow checking. * src/conf/domain_conf.h (_virDomainDef): Always use 64-bit values for memory. Change hugepage_backed to a bool. * src/conf/domain_conf.c (virDomainDefParseXML) (virDomainDefCheckABIStability, virDomainDefFormatInternal): Fix clients. * src/vmx/vmx.c (virVMXFormatConfig): Likewise. * src/xenxs/xen_sxpr.c (xenParseSxpr, xenFormatSxpr): Likewise. * src/xenxs/xen_xm.c (xenXMConfigGetULongLong): New function. (xenXMConfigGetULong, xenXMConfigSetInt): Avoid truncation. (xenParseXM, xenFormatXM): Fix clients. * src/phyp/phyp_driver.c (phypBuildLpar): Likewise. * src/openvz/openvz_driver.c (openvzDomainSetMemoryInternal): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainDefineXML): Likewise. * src/qemu/qemu_command.c (qemuBuildCommandLine): Likewise. * src/qemu/qemu_process.c (qemuProcessStart): Likewise. * src/qemu/qemu_monitor.h (qemuMonitorGetBalloonInfo): Likewise. * src/qemu/qemu_monitor_text.h (qemuMonitorTextGetBalloonInfo): Likewise. * src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBalloonInfo): Likewise. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONGetBalloonInfo): Likewise. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONGetBalloonInfo): Likewise. * src/qemu/qemu_driver.c (qemudDomainGetInfo) (qemuDomainGetXMLDesc): Likewise. * src/uml/uml_conf.c (umlBuildCommandLine): Likewise. --- v2: new src/conf/domain_audit.c | 2 +- src/conf/domain_conf.c | 46 ++++++++++++++++---------------- src/conf/domain_conf.h | 14 +++++----- src/openvz/openvz_driver.c | 8 +++--- src/phyp/phyp_driver.c | 10 +++--- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_monitor_text.c | 2 +- src/qemu/qemu_monitor_text.h | 2 +- src/qemu/qemu_process.c | 6 ++++ src/uml/uml_conf.c | 2 +- src/vbox/vbox_tmpl.c | 4 +- src/vmx/vmx.c | 10 +++--- src/xenxs/xen_sxpr.c | 8 ++--- src/xenxs/xen_xm.c | 58 +++++++++++++++++++++++++++++++++++------ 19 files changed, 115 insertions(+), 71 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 934e546..b885906 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -1,7 +1,7 @@ /* * domain_audit.c: Domain audit management * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 41d0eca..fa44d3e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7292,27 +7292,27 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; /* Extract domain memory */ - if (virXPathULong("string(./memory[1])", ctxt, - &def->mem.max_balloon) < 0) { + if (virXPathULongLong("string(./memory[1])", ctxt, + &def->mem.max_balloon) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing memory element")); goto error; } - if (virXPathULong("string(./currentMemory[1])", ctxt, - &def->mem.cur_balloon) < 0) + if (virXPathULongLong("string(./currentMemory[1])", ctxt, + &def->mem.cur_balloon) < 0) def->mem.cur_balloon = def->mem.max_balloon; if (def->mem.cur_balloon > def->mem.max_balloon) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("current memory '%luk' exceeds maximum '%luk'"), + _("current memory '%lluk' exceeds maximum '%lluk'"), def->mem.cur_balloon, def->mem.max_balloon); goto error; } node = virXPathNode("./memoryBacking/hugepages", ctxt); if (node) - def->mem.hugepage_backed = 1; + def->mem.hugepage_backed = true; /* Extract blkio cgroup tunables */ if (virXPathUInt("string(./blkiotune/weight)", ctxt, @@ -7346,20 +7346,20 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, VIR_FREE(nodes); /* Extract other memory tunables */ - if (virXPathULong("string(./memtune/hard_limit)", ctxt, - &def->mem.hard_limit) < 0) + if (virXPathULongLong("string(./memtune/hard_limit)", ctxt, + &def->mem.hard_limit) < 0) def->mem.hard_limit = 0; - if (virXPathULong("string(./memtune/soft_limit[1])", ctxt, - &def->mem.soft_limit) < 0) + if (virXPathULongLong("string(./memtune/soft_limit[1])", ctxt, + &def->mem.soft_limit) < 0) def->mem.soft_limit = 0; - if (virXPathULong("string(./memtune/min_guarantee[1])", ctxt, - &def->mem.min_guarantee) < 0) + if (virXPathULongLong("string(./memtune/min_guarantee[1])", ctxt, + &def->mem.min_guarantee) < 0) def->mem.min_guarantee = 0; - if (virXPathULong("string(./memtune/swap_hard_limit[1])", ctxt, - &def->mem.swap_hard_limit) < 0) + if (virXPathULongLong("string(./memtune/swap_hard_limit[1])", ctxt, + &def->mem.swap_hard_limit) < 0) def->mem.swap_hard_limit = 0; n = virXPathULong("string(./vcpu[1])", ctxt, &count); @@ -9210,19 +9210,19 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, if (src->mem.max_balloon != dst->mem.max_balloon) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain max memory %ld does not match source %ld"), + _("Target domain max memory %lld does not match source %lld"), dst->mem.max_balloon, src->mem.max_balloon); goto cleanup; } if (src->mem.cur_balloon != dst->mem.cur_balloon) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain current memory %ld does not match source %ld"), + _("Target domain current memory %lld does not match source %lld"), dst->mem.cur_balloon, src->mem.cur_balloon); goto cleanup; } if (src->mem.hugepage_backed != dst->mem.hugepage_backed) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain huge page backing %ld does not match source %ld"), + _("Target domain huge page backing %d does not match source %d"), dst->mem.hugepage_backed, src->mem.hugepage_backed); goto cleanup; @@ -11645,9 +11645,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, xmlIndentTreeOutput = oldIndentTreeOutput; } - virBufferAsprintf(buf, " <memory unit='KiB'>%lu</memory>\n", + virBufferAsprintf(buf, " <memory unit='KiB'>%llu</memory>\n", def->mem.max_balloon); - virBufferAsprintf(buf, " <currentMemory unit='KiB'>%lu</currentMemory>\n", + virBufferAsprintf(buf, " <currentMemory unit='KiB'>%llu</currentMemory>\n", def->mem.cur_balloon); /* add blkiotune only if there are any */ @@ -11689,19 +11689,19 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, " <memtune>\n"); if (def->mem.hard_limit) { virBufferAsprintf(buf, " <hard_limit unit='KiB'>" - "%lu</hard_limit>\n", def->mem.hard_limit); + "%llu</hard_limit>\n", def->mem.hard_limit); } if (def->mem.soft_limit) { virBufferAsprintf(buf, " <soft_limit unit='KiB'>" - "%lu</soft_limit>\n", def->mem.soft_limit); + "%llu</soft_limit>\n", def->mem.soft_limit); } if (def->mem.min_guarantee) { virBufferAsprintf(buf, " <min_guarantee unit='KiB'>" - "%lu</min_guarantee>\n", def->mem.min_guarantee); + "%llu</min_guarantee>\n", def->mem.min_guarantee); } if (def->mem.swap_hard_limit) { virBufferAsprintf(buf, " <swap_hard_limit unit='KiB'>" - "%lu</swap_hard_limit>\n", def->mem.swap_hard_limit); + "%llu</swap_hard_limit>\n", def->mem.swap_hard_limit); } if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || def->mem.swap_hard_limit) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2aa489d..f565169 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1454,13 +1454,13 @@ struct _virDomainDef { } blkio; struct { - unsigned long max_balloon; /* in kibibytes */ - unsigned long cur_balloon; /* in kibibytes */ - unsigned long hugepage_backed; - unsigned long hard_limit; /* in kibibytes */ - unsigned long soft_limit; /* in kibibytes */ - unsigned long min_guarantee; /* in kibibytes */ - unsigned long swap_hard_limit; /* in kibibytes */ + unsigned long long max_balloon; /* in kibibytes */ + unsigned long long cur_balloon; /* in kibibytes */ + bool hugepage_backed; + unsigned long long hard_limit; /* in kibibytes */ + unsigned long long soft_limit; /* in kibibytes */ + unsigned long long min_guarantee; /* in kibibytes */ + unsigned long long swap_hard_limit; /* in kibibytes */ } mem; unsigned short vcpus; unsigned short maxvcpus; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index aef1491..4e369ea 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1,7 +1,7 @@ /* * openvz_driver.c: core driver methods for managing OpenVZ VEs * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * Copyright (C) 2007 Anoop Joe Cyriac @@ -70,7 +70,7 @@ static int openvzDomainGetMaxVcpus(virDomainPtr dom); static int openvzDomainSetVcpusInternal(virDomainObjPtr vm, unsigned int nvcpus); static int openvzDomainSetMemoryInternal(virDomainObjPtr vm, - unsigned long memory); + unsigned long long memory); static int openvzGetVEStatus(virDomainObjPtr vm, int *status, int *reason); static void openvzDriverLock(struct openvz_driver *driver) @@ -1612,7 +1612,7 @@ static int openvzNumDefinedDomains(virConnectPtr conn) { static int openvzDomainSetMemoryInternal(virDomainObjPtr vm, - unsigned long mem) + unsigned long long mem) { char str_mem[16]; const char *prog[] = { VZCTL, "--quiet", "set", PROGRAM_SENTINAL, @@ -1620,7 +1620,7 @@ openvzDomainSetMemoryInternal(virDomainObjPtr vm, }; /* memory has to be changed its format from kbyte to byte */ - snprintf(str_mem, sizeof(str_mem), "%lu", mem * 1024); + snprintf(str_mem, sizeof(str_mem), "%llu", mem * 1024); openvzSetProgramSentinal(prog, vm->def->name); if (virRun(prog, NULL) < 0) { diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index e08b1b6..bdf4a7b 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright IBM Corp. 2009 * * phyp_driver.c: ssh layer to access Power Hypervisors @@ -3608,10 +3608,10 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) virBufferAddLit(&buf, "mksyscfg"); if (system_type == HMC) virBufferAsprintf(&buf, " -m %s", managed_system); - virBufferAsprintf(&buf, " -r lpar -p %s -i min_mem=%d,desired_mem=%d," - "max_mem=%d,desired_procs=%d,virtual_scsi_adapters=%s", - def->name, (int) def->mem.cur_balloon, - (int) def->mem.cur_balloon, (int) def->mem.max_balloon, + virBufferAsprintf(&buf, " -r lpar -p %s -i min_mem=%lld,desired_mem=%lld," + "max_mem=%lld,desired_procs=%d,virtual_scsi_adapters=%s", + def->name, def->mem.cur_balloon, + def->mem.cur_balloon, def->mem.max_balloon, (int) def->vcpus, def->disks[0]->src); ret = phypExecBuffer(session, &buf, &exit_status, conn, false); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cddd6aa..e6a646c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4011,7 +4011,7 @@ qemuBuildCommandLine(virConnectPtr conn, * is not supported, then they're out of luck anyway */ virCommandAddArg(cmd, "-m"); - virCommandAddArgFormat(cmd, "%lu", VIR_DIV_UP(def->mem.max_balloon, 1024)); + virCommandAddArgFormat(cmd, "%llu", VIR_DIV_UP(def->mem.max_balloon, 1024)); if (def->mem.hugepage_backed) { if (!driver->hugetlbfs_mount) { qemuReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aee95e4..bed1a7c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2139,7 +2139,7 @@ static int qemudDomainGetInfo(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; int err; - unsigned long balloon; + unsigned long long balloon; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4416,7 +4416,7 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom, struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; char *ret = NULL; - unsigned long balloon; + unsigned long long balloon; int err = 0; /* Flags checked by virDomainDefFormat */ diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1da73f6..78eb492 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1260,7 +1260,7 @@ int qemuMonitorGetVirtType(qemuMonitorPtr mon, int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, - unsigned long *currmem) + unsigned long long *currmem) { int ret; VIR_DEBUG("mon=%p", mon); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b1c956c..654d9bd 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -234,7 +234,7 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, int qemuMonitorGetVirtType(qemuMonitorPtr mon, int *virtType); int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, - unsigned long *currmem); + unsigned long long *currmem); int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index dc67b4b..afd744a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1178,7 +1178,7 @@ cleanup: * or -1 on failure */ int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, - unsigned long *currmem) + unsigned long long *currmem) { int ret; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-balloon", diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 0932a2c..04e9d86 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -61,7 +61,7 @@ int qemuMonitorJSONGetCPUInfo(qemuMonitorPtr mon, int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, int *virtType); int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, - unsigned long *currmem); + unsigned long long *currmem); int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index a7ebfba..30a0416 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -705,7 +705,7 @@ static int qemuMonitorParseBalloonInfo(char *text, * or -1 on failure */ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, - unsigned long *currmem) + unsigned long long *currmem) { char *reply = NULL; int ret = -1; diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 719fc82..4525864 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -54,7 +54,7 @@ int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon, int qemuMonitorTextGetVirtType(qemuMonitorPtr mon, int *virtType); int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, - unsigned long *currmem); + unsigned long long *currmem); int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7b99814..5d7cf4d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3488,6 +3488,12 @@ int qemuProcessStart(virConnectPtr conn, * migration completes. */ VIR_DEBUG("Setting initial memory amount"); cur_balloon = vm->def->mem.cur_balloon; + if (cur_balloon != vm->def->mem.cur_balloon) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("integer overflow trying to set balloon to %lld"), + vm->def->mem.cur_balloon); + goto cleanup; + } qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 89fdd9f..da38cc2 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -416,7 +416,7 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn, virCommandAddEnvPassCommon(cmd); //virCommandAddArgPair(cmd, "con0", "fd:0,fd:1"); - virCommandAddArgFormat(cmd, "mem=%luK", vm->def->mem.cur_balloon); + virCommandAddArgFormat(cmd, "mem=%lluK", vm->def->mem.cur_balloon); virCommandAddArgPair(cmd, "umid", vm->def->name); virCommandAddArgPair(cmd, "uml_dir", driver->monitorDir); diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index a39b567..853a599 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -8,7 +8,7 @@ */ /* - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2008-2009 Sun Microsystems, Inc. * * This file is part of a free software library; you can redistribute @@ -5052,7 +5052,7 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) { VIR_DIV_UP(def->mem.cur_balloon, 1024)); if (NS_FAILED(rc)) { vboxError(VIR_ERR_INTERNAL_ERROR, - _("could not set the memory size of the domain to: %lu Kb, " + _("could not set the memory size of the domain to: %llu Kb, " "rc=%08x"), def->mem.cur_balloon, (unsigned)rc); } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 75cb6d1..cc426da 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2,7 +2,7 @@ /* * vmx.c: VMware VMX parsing/formatting functions * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2009-2010 Matthias Bolte <matthias.bolte@googlemail.com> * * This library is free software; you can redistribute it and/or @@ -2883,7 +2883,7 @@ virVMXFormatConfig(virVMXContext *ctx, virCapsPtr caps, virDomainDefPtr def, char *preliminaryDisplayName = NULL; char *displayName = NULL; char *annotation = NULL; - unsigned long max_balloon; + unsigned long long max_balloon; bool scsi_present[4] = { false, false, false, false }; int scsi_virtualDev[4] = { -1, -1, -1, -1 }; bool floppy_present[2] = { false, false }; @@ -2978,19 +2978,19 @@ virVMXFormatConfig(virVMXContext *ctx, virCapsPtr caps, virDomainDefPtr def, /* max-memory must be a multiple of 4096 kilobyte */ max_balloon = VIR_DIV_UP(def->mem.max_balloon, 4096) * 4096; - virBufferAsprintf(&buffer, "memsize = \"%lu\"\n", + virBufferAsprintf(&buffer, "memsize = \"%llu\"\n", max_balloon / 1024); /* Scale from kilobytes to megabytes */ /* def:mem.cur_balloon -> vmx:sched.mem.max */ if (def->mem.cur_balloon < max_balloon) { - virBufferAsprintf(&buffer, "sched.mem.max = \"%lu\"\n", + virBufferAsprintf(&buffer, "sched.mem.max = \"%llu\"\n", VIR_DIV_UP(def->mem.cur_balloon, 1024)); /* Scale from kilobytes to megabytes */ } /* def:mem.min_guarantee -> vmx:sched.mem.minsize */ if (def->mem.min_guarantee > 0) { - virBufferAsprintf(&buffer, "sched.mem.minsize = \"%lu\"\n", + virBufferAsprintf(&buffer, "sched.mem.minsize = \"%llu\"\n", VIR_DIV_UP(def->mem.min_guarantee, 1024)); /* Scale from kilobytes to megabytes */ } diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index f8390ea..beafb95 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1190,10 +1190,8 @@ xenParseSxpr(const struct sexpr *root, } } - def->mem.max_balloon = (unsigned long) - (sexpr_u64(root, "domain/maxmem") << 10); - def->mem.cur_balloon = (unsigned long) - (sexpr_u64(root, "domain/memory") << 10); + def->mem.max_balloon = (sexpr_u64(root, "domain/maxmem") << 10); + def->mem.cur_balloon = (sexpr_u64(root, "domain/memory") << 10); if (def->mem.cur_balloon > def->mem.max_balloon) def->mem.cur_balloon = def->mem.max_balloon; @@ -2201,7 +2199,7 @@ xenFormatSxpr(virConnectPtr conn, virBufferAddLit(&buf, "(vm "); virBufferEscapeSexpr(&buf, "(name '%s')", def->name); - virBufferAsprintf(&buf, "(memory %lu)(maxmem %lu)", + virBufferAsprintf(&buf, "(memory %llu)(maxmem %llu)", VIR_DIV_UP(def->mem.cur_balloon, 1024), VIR_DIV_UP(def->mem.max_balloon, 1024)); virBufferAsprintf(&buf, "(vcpus %u)", def->maxvcpus); diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index e580a3e..6676e52 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1,8 +1,8 @@ /* * xen_xm.c: Xen XM parsing functions * + * Copyright (C) 2006-2007, 2009-2010, 2012 Red Hat, Inc. * Copyright (C) 2011 Univention GmbH - * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -68,7 +68,7 @@ static int xenXMConfigGetBool(virConfPtr conf, static int xenXMConfigGetULong(virConfPtr conf, const char *name, unsigned long *value, - int def) { + unsigned long def) { virConfValuePtr val; *value = 0; @@ -96,6 +96,38 @@ static int xenXMConfigGetULong(virConfPtr conf, } +/* Convenience method to grab a int from the config file object */ +static int xenXMConfigGetULongLong(virConfPtr conf, + const char *name, + unsigned long long *value, + unsigned long long def) { + virConfValuePtr val; + + *value = 0; + if (!(val = virConfGetValue(conf, name))) { + *value = def; + return 0; + } + + if (val->type == VIR_CONF_LONG) { + *value = val->l; + } else if (val->type == VIR_CONF_STRING) { + char *ret; + *value = strtoll(val->str, &ret, 10); + if (ret == val->str) { + XENXS_ERROR(VIR_ERR_INTERNAL_ERROR, + _("config value %s was malformed"), name); + return -1; + } + } else { + XENXS_ERROR(VIR_ERR_INTERNAL_ERROR, + _("config value %s was malformed"), name); + return -1; + } + return 0; +} + + /* Convenience method to grab a string from the config file object */ static int xenXMConfigGetString(virConfPtr conf, const char *name, @@ -312,12 +344,12 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, goto cleanup; } - if (xenXMConfigGetULong(conf, "memory", &def->mem.cur_balloon, - MIN_XEN_GUEST_SIZE * 2) < 0) + if (xenXMConfigGetULongLong(conf, "memory", &def->mem.cur_balloon, + MIN_XEN_GUEST_SIZE * 2) < 0) goto cleanup; - if (xenXMConfigGetULong(conf, "maxmem", &def->mem.max_balloon, - def->mem.cur_balloon) < 0) + if (xenXMConfigGetULongLong(conf, "maxmem", &def->mem.max_balloon, + def->mem.cur_balloon) < 0) goto cleanup; def->mem.cur_balloon *= 1024; @@ -1101,9 +1133,15 @@ cleanup: static -int xenXMConfigSetInt(virConfPtr conf, const char *setting, long l) { +int xenXMConfigSetInt(virConfPtr conf, const char *setting, long long l) { virConfValuePtr value = NULL; + if ((long) l != l) { + XENXS_ERROR(VIR_ERR_INTERNAL_ERROR, + _("integer overflow in trying to store %lld to %s"), + l, setting); + return -1; + } if (VIR_ALLOC(value) < 0) { virReportOOMError(); return -1; @@ -1468,10 +1506,12 @@ virConfPtr xenFormatXM(virConnectPtr conn, if (xenXMConfigSetString(conf, "uuid", uuid) < 0) goto no_memory; - if (xenXMConfigSetInt(conf, "maxmem", VIR_DIV_UP(def->mem.max_balloon, 1024)) < 0) + if (xenXMConfigSetInt(conf, "maxmem", + VIR_DIV_UP(def->mem.max_balloon, 1024)) < 0) goto no_memory; - if (xenXMConfigSetInt(conf, "memory", VIR_DIV_UP(def->mem.cur_balloon, 1024)) < 0) + if (xenXMConfigSetInt(conf, "memory", + VIR_DIV_UP(def->mem.cur_balloon, 1024)) < 0) goto no_memory; if (xenXMConfigSetInt(conf, "vcpus", def->maxvcpus) < 0) -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Using 'unsigned long' for memory values is risky on 32-bit platforms, as a PAE guest can have more than 4GiB memory. Our API is (unfortunately) locked at 'unsigned long' and a scale of 1024, but the rest of our system should consistently use 64-bit values, especially since the previous patch centralized overflow checking.
* src/conf/domain_conf.h (_virDomainDef): Always use 64-bit values for memory. Change hugepage_backed to a bool. * src/conf/domain_conf.c (virDomainDefParseXML) (virDomainDefCheckABIStability, virDomainDefFormatInternal): Fix clients. * src/vmx/vmx.c (virVMXFormatConfig): Likewise. * src/xenxs/xen_sxpr.c (xenParseSxpr, xenFormatSxpr): Likewise. * src/xenxs/xen_xm.c (xenXMConfigGetULongLong): New function. (xenXMConfigGetULong, xenXMConfigSetInt): Avoid truncation. (xenParseXM, xenFormatXM): Fix clients. * src/phyp/phyp_driver.c (phypBuildLpar): Likewise. * src/openvz/openvz_driver.c (openvzDomainSetMemoryInternal): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainDefineXML): Likewise. * src/qemu/qemu_command.c (qemuBuildCommandLine): Likewise. * src/qemu/qemu_process.c (qemuProcessStart): Likewise. * src/qemu/qemu_monitor.h (qemuMonitorGetBalloonInfo): Likewise. * src/qemu/qemu_monitor_text.h (qemuMonitorTextGetBalloonInfo): Likewise. * src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBalloonInfo): Likewise. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONGetBalloonInfo): Likewise. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONGetBalloonInfo): Likewise. * src/qemu/qemu_driver.c (qemudDomainGetInfo) (qemuDomainGetXMLDesc): Likewise. * src/uml/uml_conf.c (umlBuildCommandLine): Likewise. --- diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index f8390ea..beafb95 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1101,9 +1133,15 @@ cleanup:
static -int xenXMConfigSetInt(virConfPtr conf, const char *setting, long l) { +int xenXMConfigSetInt(virConfPtr conf, const char *setting, long long l) { virConfValuePtr value = NULL;
+ if ((long) l != l) { + XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
I suppose the VIR_ERR_INTERNAL_ERROR is intentional.
+ _("integer overflow in trying to store %lld to %s"), + l, setting); + return -1; + } if (VIR_ALLOC(value)< 0) { virReportOOMError(); return -1;
ACK, Peter

On 03/06/2012 09:34 AM, Peter Krempa wrote:
On 03/06/2012 01:34 AM, Eric Blake wrote:
Using 'unsigned long' for memory values is risky on 32-bit platforms, as a PAE guest can have more than 4GiB memory. Our API is (unfortunately) locked at 'unsigned long' and a scale of 1024, but the rest of our system should consistently use 64-bit values, especially since the previous patch centralized overflow checking.
+++ b/src/xenxs/xen_sxpr.c @@ -1101,9 +1133,15 @@ cleanup:
static -int xenXMConfigSetInt(virConfPtr conf, const char *setting, long l) { +int xenXMConfigSetInt(virConfPtr conf, const char *setting, long long l) { virConfValuePtr value = NULL;
+ if ((long) l != l) { + XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
I suppose the VIR_ERR_INTERNAL_ERROR is intentional.
Yes - the libvirt.c interface takes 'unsigned long', and widening that to long long should be reversible so we should never hit this error.
+ _("integer overflow in trying to store %lld to %s"), + l, setting); + return -1; + } if (VIR_ALLOC(value)< 0) { virReportOOMError(); return -1;
ACK,
Peter
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Output is still in kibibytes, but input can now be in different scales for ease of typing. * src/conf/domain_conf.c (virDomainParseMemory): New helper. (virDomainDefParseXML): Use it when parsing. * docs/schemas/domaincommon.rng: Expand XML; rename memoryKBElement to memoryElement and update callers. * docs/formatdomain.html.in (elementsMemoryAllocation): Document scaling. * tests/qemuxml2argvdata/qemuxml2argv-memtune.xml: Adjust test. * tests/qemuxml2xmltest.c: Likewise. * tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml: New file. --- v2: reuse code introduced earlier in series, add tests docs/formatdomain.html.in | 42 +++++++-- docs/schemas/domaincommon.rng | 23 ++---- src/conf/domain_conf.c | 93 +++++++++++++++---- tests/qemuxml2argvdata/qemuxml2argv-memtune.xml | 6 +- .../qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml | 31 +++++++ tests/qemuxml2xmltest.c | 2 +- 6 files changed, 147 insertions(+), 50 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0855c7f..1e4a990 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -415,8 +415,8 @@ <pre> <domain> ... - <memory>524288</memory> - <currentMemory>524288</currentMemory> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> ... </domain> </pre> @@ -424,12 +424,30 @@ <dl> <dt><code>memory</code></dt> <dd>The maximum allocation of memory for the guest at boot time. - The units for this value are kibibytes (i.e. blocks of 1024 bytes)</dd> + The units for this value are determined by the optional + atttribute <code>unit</code>, which defaults to "KiB" + (kibibytes, 2<sup>10</sup> or blocks of 1024 bytes). Valid + units are "b" or "bytes" for bytes, "KB" for kilobytes + (10<sup>3</sup> or 1,000), "k" or "KiB" for kibibytes (1024), + "MB" for megabytes (10<sup>6</sup> or 1,000,000), "M" or "MiB" + for mebibytes (2<sup>20</sup> or 1,048,576), "GB" for + gigabytes (10<sup>9</sup> or 1,000,000,000), "G" or "GiB" for + gibibytes (2<sup>30</sup> or 1,073,741,824), "TB" for + terabytes (10<sup>12</sup> or 1,000,000,000,000), or "T" or + "TiB" for tebibytes (2<sup>40</sup> or 1,099,511,627,776). + However, the value will be rounded up to the nearest kibibyte + by libvirt, and may be further rounded to the granularity + supported by the hypervisor. Some hypervisors also enforce a + minimum, such as + 4000KiB. <span class='since'><code>unit</code> since + 0.9.11</span></dd> <dt><code>currentMemory</code></dt> <dd>The actual allocation of memory for the guest. This value can be less than the maximum allocation, to allow for ballooning up the guests memory on the fly. If this is omitted, it defaults - to the same value as the <code>memory</code> element</dd> + to the same value as the <code>memory</code> element. + The <code>unit</code> attribute behaves the same as + for <code>memory</code>.</dd> </dl> @@ -460,10 +478,10 @@ <domain> ... <memtune> - <hard_limit>1048576</hard_limit> - <soft_limit>131072</soft_limit> - <swap_hard_limit>2097152</swap_hard_limit> - <min_guarantee>65536</min_guarantee> + <hard_limit unit='G'>1</hard_limit> + <soft_limit unit='M'>128</soft_limit> + <swap_hard_limit unit='G'>2</swap_hard_limit> + <min_guarantee unit='bytes'>67108864</min_guarantee> </memtune> ... </domain> @@ -477,7 +495,13 @@ parameters are applied to the QEMU process as a whole. Thus, when counting them, one needs to add up guest RAM, guest video RAM, and some memory overhead of QEMU itself. The last piece is hard to - determine so one needs guess and try.</dd> + determine so one needs guess and try. For each tunable, it + is possible to designate which unit the number is in on + input, using the same values as + for <code><memory></code>. For backwards + compatibility, output is always in + KiB. <span class='since'><code>unit</code> + since 0.9.11</span></dd> <dt><code>hard_limit</code></dt> <dd> The optional <code>hard_limit</code> element is the maximum memory the guest can use. The units for this value are kibibytes (i.e. blocks diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 99b3861..dfa1d1e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -412,11 +412,11 @@ <define name="resources"> <interleave> <element name="memory"> - <ref name='memoryKBElement'/> + <ref name='scaledInteger'/> </element> <optional> <element name="currentMemory"> - <ref name='memoryKBElement'/> + <ref name='scaledInteger'/> </element> </optional> <optional> @@ -461,25 +461,25 @@ <!-- Maximum memory the VM can use --> <optional> <element name="hard_limit"> - <ref name='memoryKBElement'/> + <ref name='scaledInteger'/> </element> </optional> <!-- Minimum memory ascertained for the VM during contention --> <optional> <element name="soft_limit"> - <ref name='memoryKBElement'/> + <ref name='scaledInteger'/> </element> </optional> <!-- Minimum amount of memory required to start the VM --> <optional> <element name="min_guarantee"> - <ref name='memoryKBElement'/> + <ref name='scaledInteger'/> </element> </optional> <!-- Maximum swap area the VM can use --> <optional> <element name="swap_hard_limit"> - <ref name='memoryKBElement'/> + <ref name='scaledInteger'/> </element> </optional> </element> @@ -3082,17 +3082,6 @@ <param name="pattern">[0-9]+</param> </data> </define> - <!-- Memory as an element, with optional unit attribute --> - <define name='memoryKBElement'> - <optional> - <attribute name='unit'> - <value>KiB</value> - </attribute> - </optional> - <data type='unsignedInt'> - <param name='pattern'>[0-9]+</param> - </data> - </define> <define name="domainName"> <data type="string"> <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fa44d3e..3bc2fae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7169,6 +7169,60 @@ static int virDomainDefMaybeAddController(virDomainDefPtr def, return 0; } + +/* Parse a memory element located at XPATH within CTXT, and store the + * result into MEM. If REQUIRED, then the value must exist; + * otherwise, the value is optional. The value is in blocks of 1024. + * Return 0 on success, -1 on failure after issuing error. */ +static int +virDomainParseMemory(const char *xpath, xmlXPathContextPtr ctxt, + unsigned long long *mem, bool required) +{ + char *xpath_full = NULL; + char *unit = NULL; + int ret = -1; + unsigned long long bytes; + unsigned long long max; + + *mem = 0; + if (virAsprintf(&xpath_full, "string(%s)", xpath) < 0) { + virReportOOMError(); + goto cleanup; + } + if (virXPathULongLong(xpath_full, ctxt, &bytes) < 0) { + if (required) + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing memory element")); + else + ret = 0; + goto cleanup; + } + VIR_FREE(xpath_full); + + if (virAsprintf(&xpath_full, "string(%s/@unit)", xpath) < 0) { + virReportOOMError(); + goto cleanup; + } + unit = virXPathString(xpath_full, ctxt); + /* On 32-bit machines, our bound is 0xffffffff * KiB. On 64-bit + * machines, our bound is off_t (2^63). */ + if (sizeof(unsigned long) < sizeof(long long)) + max = 1024ull * ULONG_MAX; + else + max = LLONG_MAX; + if (virScaleInteger(&bytes, unit, 1024, max) < 0) + goto cleanup; + + /* Yes, we really do use kibibytes for our internal sizing. */ + *mem = VIR_DIV_UP(bytes, 1024); + ret = 0; +cleanup: + VIR_FREE(xpath_full); + VIR_FREE(unit); + return ret; +} + + static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, xmlDocPtr xml, xmlNodePtr root, @@ -7292,22 +7346,21 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; /* Extract domain memory */ - if (virXPathULongLong("string(./memory[1])", ctxt, - &def->mem.max_balloon) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing memory element")); + if (virDomainParseMemory("./memory[1]", ctxt, + &def->mem.max_balloon, true) < 0) goto error; - } - if (virXPathULongLong("string(./currentMemory[1])", ctxt, - &def->mem.cur_balloon) < 0) - def->mem.cur_balloon = def->mem.max_balloon; + if (virDomainParseMemory("./currentMemory[1]", ctxt, + &def->mem.cur_balloon, false) < 0) + goto error; if (def->mem.cur_balloon > def->mem.max_balloon) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("current memory '%lluk' exceeds maximum '%lluk'"), def->mem.cur_balloon, def->mem.max_balloon); goto error; + } else if (def->mem.cur_balloon == 0) { + def->mem.cur_balloon = def->mem.max_balloon; } node = virXPathNode("./memoryBacking/hugepages", ctxt); @@ -7346,21 +7399,21 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, VIR_FREE(nodes); /* Extract other memory tunables */ - if (virXPathULongLong("string(./memtune/hard_limit)", ctxt, - &def->mem.hard_limit) < 0) - def->mem.hard_limit = 0; + if (virDomainParseMemory("./memtune/hard_limit[1]", ctxt, + &def->mem.hard_limit, false) < 0) + goto error; - if (virXPathULongLong("string(./memtune/soft_limit[1])", ctxt, - &def->mem.soft_limit) < 0) - def->mem.soft_limit = 0; + if (virDomainParseMemory("./memtune/soft_limit[1]", ctxt, + &def->mem.soft_limit, false) < 0) + goto error; - if (virXPathULongLong("string(./memtune/min_guarantee[1])", ctxt, - &def->mem.min_guarantee) < 0) - def->mem.min_guarantee = 0; + if (virDomainParseMemory("./memtune/min_guarantee[1]", ctxt, + &def->mem.min_guarantee, false) < 0) + goto error; - if (virXPathULongLong("string(./memtune/swap_hard_limit[1])", ctxt, - &def->mem.swap_hard_limit) < 0) - def->mem.swap_hard_limit = 0; + if (virDomainParseMemory("./memtune/swap_hard_limit[1]", ctxt, + &def->mem.swap_hard_limit, false) < 0) + goto error; n = virXPathULong("string(./vcpu[1])", ctxt, &count); if (n == -2) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml b/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml index b6c8c95..ee08753 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml @@ -1,12 +1,12 @@ <domain type='qemu'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219136</memory> + <memory unit='MiB'>214</memory> <currentMemory unit='KiB'>219136</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> + <soft_limit unit='bytes'>131071999</soft_limit> + <swap_hard_limit unit='KB'>1048576</swap_hard_limit> </memtune> <vcpu>1</vcpu> <os> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml new file mode 100644 index 0000000..b6c8c95 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml @@ -0,0 +1,31 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4a2e016..629737f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -194,7 +194,7 @@ mymain(void) DO_TEST("pci-rom"); DO_TEST("encrypted-disk"); - DO_TEST("memtune"); + DO_TEST_DIFFERENT("memtune"); DO_TEST("blkiotune"); DO_TEST("blkiotune-device"); DO_TEST("cputune"); -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Output is still in kibibytes, but input can now be in different scales for ease of typing.
* src/conf/domain_conf.c (virDomainParseMemory): New helper. (virDomainDefParseXML): Use it when parsing. * docs/schemas/domaincommon.rng: Expand XML; rename memoryKBElement to memoryElement and update callers. * docs/formatdomain.html.in (elementsMemoryAllocation): Document scaling. * tests/qemuxml2argvdata/qemuxml2argv-memtune.xml: Adjust test. * tests/qemuxml2xmltest.c: Likewise. * tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml: New file. ---
v2: reuse code introduced earlier in series, add tests
docs/formatdomain.html.in | 42 +++++++-- docs/schemas/domaincommon.rng | 23 ++---- src/conf/domain_conf.c | 93 +++++++++++++++---- tests/qemuxml2argvdata/qemuxml2argv-memtune.xml | 6 +- .../qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml | 31 +++++++ tests/qemuxml2xmltest.c | 2 +- 6 files changed, 147 insertions(+), 50 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0855c7f..1e4a990 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -415,8 +415,8 @@ <pre> <domain> ... -<memory>524288</memory> -<currentMemory>524288</currentMemory> +<memory unit='KiB'>524288</memory> +<currentMemory unit='KiB'>524288</currentMemory> ... </domain> </pre> @@ -424,12 +424,30 @@ <dl> <dt><code>memory</code></dt> <dd>The maximum allocation of memory for the guest at boot time. - The units for this value are kibibytes (i.e. blocks of 1024 bytes)</dd> + The units for this value are determined by the optional + atttribute<code>unit</code>, which defaults to "KiB" + (kibibytes, 2<sup>10</sup> or blocks of 1024 bytes). Valid + units are "b" or "bytes" for bytes, "KB" for kilobytes + (10<sup>3</sup> or 1,000), "k" or "KiB" for kibibytes (1024), + "MB" for megabytes (10<sup>6</sup> or 1,000,000), "M" or "MiB" + for mebibytes (2<sup>20</sup> or 1,048,576), "GB" for
As in the previous similar sections, I'd explicitly specify that the numbers are bytes here.
+ gigabytes (10<sup>9</sup> or 1,000,000,000), "G" or "GiB" for + gibibytes (2<sup>30</sup> or 1,073,741,824), "TB" for + terabytes (10<sup>12</sup> or 1,000,000,000,000), or "T" or + "TiB" for tebibytes (2<sup>40</sup> or 1,099,511,627,776). + However, the value will be rounded up to the nearest kibibyte + by libvirt, and may be further rounded to the granularity + supported by the hypervisor. Some hypervisors also enforce a + minimum, such as
This patch clears everything I was afraid in 05/15. ACK, Peter

In the past, we have created some virsh options with less-than-stellar names. For back-compat reasons, those names must continue to parse, but we don't want to document them in help output. This introduces a new option type, an alias, which points to a canonical option name later in the option list. I'm actually quite impressed that our code has already been factored to do all option parsing through common entry points, such that I got this added in relatively few lines of code! * tools/virsh.c (VSH_OT_ALIAS): New option type. (opts_echo): Hook up an alias, for easy testing. (vshCmddefOptParse, vshCmddefHelp, vshCmddefGetOption): Allow for aliases. * tests/virshtest.c (mymain): Test new feature. --- v2: no real change tests/virshtest.c | 6 ++++++ tools/virsh.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/tests/virshtest.c b/tests/virshtest.c index 6474c19..87b1336 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -386,6 +386,12 @@ mymain(void) DO_TEST(30, "--shell a\n", "echo \t '-'\"-\" \t --shell \t a"); + /* Tests of alias handling. */ + DO_TEST(31, "hello\n", "echo", "--string", "hello"); + DO_TEST(32, "hello\n", "echo --string hello"); + DO_TEST(33, "hello\n", "echo", "--str", "hello"); + DO_TEST(34, "hello\n", "echo --str hello"); + # undef DO_TEST VIR_FREE(custom_uri); diff --git a/tools/virsh.c b/tools/virsh.c index aef050f..77cf4ac 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -138,7 +138,8 @@ typedef enum { VSH_OT_STRING, /* optional string option */ VSH_OT_INT, /* optional or mandatory int option */ VSH_OT_DATA, /* string data (as non-option) */ - VSH_OT_ARGV /* remaining arguments */ + VSH_OT_ARGV, /* remaining arguments */ + VSH_OT_ALIAS, /* alternate spelling for a later argument */ } vshCmdOptType; /* @@ -190,7 +191,8 @@ typedef struct { const char *name; /* the name of option, or NULL for list end */ vshCmdOptType type; /* option type */ unsigned int flags; /* flags */ - const char *help; /* non-NULL help string */ + const char *help; /* non-NULL help string; or for VSH_OT_ALIAS + * the name of a later public option */ } vshCmdOptDef; /* @@ -15209,6 +15211,7 @@ static const vshCmdInfo info_echo[] = { static const vshCmdOptDef opts_echo[] = { {"shell", VSH_OT_BOOL, 0, N_("escape for shell use")}, {"xml", VSH_OT_BOOL, 0, N_("escape for XML use")}, + {"str", VSH_OT_ALIAS, 0, "string"}, {"string", VSH_OT_ARGV, 0, N_("arguments to echo")}, {NULL, 0, 0, NULL} }; @@ -17256,6 +17259,18 @@ vshCmddefOptParse(const vshCmdDef *cmd, uint32_t *opts_need_arg, return -1; /* bool options can't be mandatory */ continue; } + if (opt->type == VSH_OT_ALIAS) { + int j; + if (opt->flags || !opt->help) + return -1; /* alias options are tracked by the original name */ + for (j = i + 1; cmd->opts[j].name; j++) { + if (STREQ(opt->help, cmd->opts[j].name)) + break; + } + if (!cmd->opts[j].name) + return -1; /* alias option must map to a later option name */ + continue; + } if (opt->flags & VSH_OFLAG_REQ_OPT) { if (opt->flags & VSH_OFLAG_REQ) *opts_required |= 1 << i; @@ -17287,6 +17302,10 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name, const vshCmdOptDef *opt = &cmd->opts[i]; if (STREQ(opt->name, name)) { + if (opt->type == VSH_OT_ALIAS) { + name = opt->help; + continue; + } if ((*opts_seen & (1 << i)) && opt->type != VSH_OT_ARGV) { vshError(ctl, _("option --%s already seen"), name); return NULL; @@ -17465,6 +17484,9 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) : _("[<%s>]..."); } break; + case VSH_OT_ALIAS: + /* aliases are intentionally undocumented */ + continue; default: assert(0); } @@ -17506,6 +17528,8 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) shortopt ? _("[--%s] <string>") : _("<%s>"), opt->name); break; + case VSH_OT_ALIAS: + continue; default: assert(0); } -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
In the past, we have created some virsh options with less-than-stellar names. For back-compat reasons, those names must continue to parse, but we don't want to document them in help output. This introduces a new option type, an alias, which points to a canonical option name later in the option list.
I'm actually quite impressed that our code has already been factored to do all option parsing through common entry points, such that I got this added in relatively few lines of code!
* tools/virsh.c (VSH_OT_ALIAS): New option type. (opts_echo): Hook up an alias, for easy testing. (vshCmddefOptParse, vshCmddefHelp, vshCmddefGetOption): Allow for aliases. * tests/virshtest.c (mymain): Test new feature. ---
Nice way to mask old mistakes and still support them. I'm wondering if this will not confuse people if their beloved arguments disappear suddenly from the docs. Maybe the help command could explicitly state aliases that exist for commands to avoid some confusion. I'm leaning towards an ACK as it's better to encourage to use the fixed spelling. Does anyone object? Peter

On 03/06/2012 10:46 AM, Peter Krempa wrote:
On 03/06/2012 01:34 AM, Eric Blake wrote:
In the past, we have created some virsh options with less-than-stellar names. For back-compat reasons, those names must continue to parse, but we don't want to document them in help output. This introduces a new option type, an alias, which points to a canonical option name later in the option list.
I'm actually quite impressed that our code has already been factored to do all option parsing through common entry points, such that I got this added in relatively few lines of code!
* tools/virsh.c (VSH_OT_ALIAS): New option type. (opts_echo): Hook up an alias, for easy testing. (vshCmddefOptParse, vshCmddefHelp, vshCmddefGetOption): Allow for aliases. * tests/virshtest.c (mymain): Test new feature. ---
Nice way to mask old mistakes and still support them.
I'm wondering if this will not confuse people if their beloved arguments disappear suddenly from the docs. Maybe the help command could explicitly state aliases that exist for commands to avoid some confusion.
I'll document the old name in the man page, but not in 'virsh help'.
I'm leaning towards an ACK as it's better to encourage to use the fixed spelling. Does anyone object?
Peter
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/06/2012 01:34 AM, Eric Blake wrote:
In the past, we have created some virsh options with less-than-stellar names. For back-compat reasons, those names must continue to parse, but we don't want to document them in help output. This introduces a new option type, an alias, which points to a canonical option name later in the option list.
I'm actually quite impressed that our code has already been factored to do all option parsing through common entry points, such that I got this added in relatively few lines of code!
* tools/virsh.c (VSH_OT_ALIAS): New option type. (opts_echo): Hook up an alias, for easy testing. (vshCmddefOptParse, vshCmddefHelp, vshCmddefGetOption): Allow for aliases. * tests/virshtest.c (mymain): Test new feature. ---
As a side note: It would be the best with this change to put a warning into the mang page that options that disappeared from the help/man page are not vanished but are obsolete and the user should use the new spellings but we still support the legacy ones. Peter

Command line interfaces should use dash, not underscore, as many keyboard layouts allow that to be typed with fewer shift key presses. Also, the US spelling of --tunneled gets more google hits than the UK spelling of --tunnelled. * tools/virsh.c (opts_migrate): Allow US variant. (opts_blkdeviotune): Prefer - over _. * tools/virsh.pod (blkdeviotune): Fix spelling. --- v2: no real change tools/virsh.c | 49 +++++++++++++++++++++++++++++++------------------ tools/virsh.pod | 18 +++++++++--------- 2 files changed, 40 insertions(+), 27 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 77cf4ac..75a1a3b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6879,6 +6879,7 @@ static const vshCmdOptDef opts_migrate[] = { {"live", VSH_OT_BOOL, 0, N_("live migration")}, {"p2p", VSH_OT_BOOL, 0, N_("peer-2-peer migration")}, {"direct", VSH_OT_BOOL, 0, N_("direct migration")}, + {"tunneled", VSH_OT_ALIAS, 0, "tunnelled"}, {"tunnelled", VSH_OT_BOOL, 0, N_("tunnelled migration")}, {"persistent", VSH_OT_BOOL, 0, N_("persist VM on destination")}, {"undefinesource", VSH_OT_BOOL, 0, N_("undefine VM on source")}, @@ -7574,17 +7575,23 @@ static const vshCmdInfo info_blkdeviotune[] = { static const vshCmdOptDef opts_blkdeviotune[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("block device")}, - {"total_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE, + {"total_bytes_sec", VSH_OT_ALIAS, 0, "total-bytes-sec"}, + {"total-bytes-sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("total throughput limit in bytes per second")}, - {"read_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE, + {"read_bytes_sec", VSH_OT_ALIAS, 0, "read-bytes-sec"}, + {"read-bytes-sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("read throughput limit in bytes per second")}, - {"write_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE, + {"write_bytes_sec", VSH_OT_ALIAS, 0, "write-bytes-sec"}, + {"write-bytes-sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("write throughput limit in bytes per second")}, - {"total_iops_sec", VSH_OT_INT, VSH_OFLAG_NONE, + {"total_iops_sec", VSH_OT_ALIAS, 0, "total-iops-sec"}, + {"total-iops-sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("total I/O operations limit per second")}, - {"read_iops_sec", VSH_OT_INT, VSH_OFLAG_NONE, + {"read_iops_sec", VSH_OT_ALIAS, 0, "read-iops-sec"}, + {"read-iops-sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("read I/O operations limit per second")}, - {"write_iops_sec", VSH_OT_INT, VSH_OFLAG_NONE, + {"write_iops_sec", VSH_OT_ALIAS, 0, "write-iops-sec"}, + {"write-iops-sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("write I/O operations limit per second")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, @@ -7630,7 +7637,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, "device", &disk) < 0) goto cleanup; - if ((rv = vshCommandOptULongLong(cmd, "total_bytes_sec", &total_bytes_sec)) < 0) { + if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec", + &total_bytes_sec)) < 0) { vshError(ctl, "%s", _("Unable to parse integer parameter")); goto cleanup; @@ -7638,7 +7646,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) nparams++; } - if ((rv = vshCommandOptULongLong(cmd, "read_bytes_sec", &read_bytes_sec)) < 0) { + if ((rv = vshCommandOptULongLong(cmd, "read-bytes-sec", + &read_bytes_sec)) < 0) { vshError(ctl, "%s", _("Unable to parse integer parameter")); goto cleanup; @@ -7646,7 +7655,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) nparams++; } - if ((rv = vshCommandOptULongLong(cmd, "write_bytes_sec", &write_bytes_sec)) < 0) { + if ((rv = vshCommandOptULongLong(cmd, "write-bytes-sec", + &write_bytes_sec)) < 0) { vshError(ctl, "%s", _("Unable to parse integer parameter")); goto cleanup; @@ -7654,7 +7664,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) nparams++; } - if ((rv = vshCommandOptULongLong(cmd, "total_iops_sec", &total_iops_sec)) < 0) { + if ((rv = vshCommandOptULongLong(cmd, "total-iops-sec", + &total_iops_sec)) < 0) { vshError(ctl, "%s", _("Unable to parse integer parameter")); goto cleanup; @@ -7662,7 +7673,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) nparams++; } - if ((rv = vshCommandOptULongLong(cmd, "read_iops_sec", &read_iops_sec)) < 0) { + if ((rv = vshCommandOptULongLong(cmd, "read-iops-sec", + &read_iops_sec)) < 0) { vshError(ctl, "%s", _("Unable to parse integer parameter")); goto cleanup; @@ -7670,7 +7682,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) nparams++; } - if ((rv = vshCommandOptULongLong(cmd, "write_iops_sec", &write_iops_sec)) < 0) { + if ((rv = vshCommandOptULongLong(cmd, "write-iops-sec", + &write_iops_sec)) < 0) { vshError(ctl, "%s", _("Unable to parse integer parameter")); goto cleanup; @@ -7712,7 +7725,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) params = vshCalloc(ctl, nparams, sizeof(*params)); i = 0; - if ((i < nparams) && (vshCommandOptBool(cmd, "total_bytes_sec"))) { + if ((i < nparams) && (vshCommandOptBool(cmd, "total-bytes-sec"))) { temp = ¶ms[i]; temp->type = VIR_TYPED_PARAM_ULLONG; strncpy(temp->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC, @@ -7721,7 +7734,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) i++; } - if ((i < nparams) && (vshCommandOptBool(cmd, "read_bytes_sec"))) { + if ((i < nparams) && (vshCommandOptBool(cmd, "read-bytes-sec"))) { temp = ¶ms[i]; temp->type = VIR_TYPED_PARAM_ULLONG; strncpy(temp->field, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC, @@ -7730,7 +7743,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) i++; } - if ((i < nparams) && (vshCommandOptBool(cmd, "write_bytes_sec"))) { + if ((i < nparams) && (vshCommandOptBool(cmd, "write-bytes-sec"))) { temp = ¶ms[i]; temp->type = VIR_TYPED_PARAM_ULLONG; strncpy(temp->field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC, @@ -7739,7 +7752,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) i++; } - if ((i < nparams) && (vshCommandOptBool(cmd, "total_iops_sec"))) { + if ((i < nparams) && (vshCommandOptBool(cmd, "total-iops-sec"))) { temp = ¶ms[i]; temp->type = VIR_TYPED_PARAM_ULLONG; strncpy(temp->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC, @@ -7748,7 +7761,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) i++; } - if ((i < nparams) && (vshCommandOptBool(cmd, "read_iops_sec"))) { + if ((i < nparams) && (vshCommandOptBool(cmd, "read-iops-sec"))) { temp = ¶ms[i]; temp->type = VIR_TYPED_PARAM_ULLONG; strncpy(temp->field, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC, @@ -7757,7 +7770,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) i++; } - if ((i < nparams) && (vshCommandOptBool(cmd, "write_iops_sec"))) { + if ((i < nparams) && (vshCommandOptBool(cmd, "write-iops-sec"))) { temp = ¶ms[i]; temp->type = VIR_TYPED_PARAM_ULLONG; strncpy(temp->field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC, diff --git a/tools/virsh.pod b/tools/virsh.pod index 8f6a2d6..1bc55c4 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -614,8 +614,8 @@ I<bandwidth> specifies copying bandwidth limit in Mbps. =item B<blkdeviotune> I<domain> I<device> [[I<--config>] [I<--live>] | [I<--current>]] -[[I<total_bytes_sec>] | [I<read_bytes_sec>] [I<write_bytes_sec>]] -[[I<total_iops_sec>] | [I<read_iops_sec>] [I<write_iops_sec>]] +[[I<total-bytes-sec>] | [I<read-bytes-sec>] [I<write-bytes-sec>]] +[[I<total-iops-sec>] | [I<read-iops-sec>] [I<write-iops-sec>]] Set or query the block disk io parameters for a block device of I<domain>. I<device> specifies a unique target name (<target dev='name'/>) or source @@ -624,15 +624,15 @@ I<domain> (see also B<domblklist> for listing these names). If no limit is specified, it will query current I/O limits setting. Otherwise, alter the limits with these flags: -I<--total_bytes_sec> specifies total throughput limit in bytes per second. -I<--read_bytes_sec> specifies read throughput limit in bytes per second. -I<--write_bytes_sec> specifies write throughput limit in bytes per second. -I<--total_iops_sec> specifies total I/O operations limit per second. -I<--read_iops_sec> specifies read I/O operations limit per second. -I<--write_iops_sec> specifies write I/O operations limit per second. +I<--total-bytes-sec> specifies total throughput limit in bytes per second. +I<--read-bytes-sec> specifies read throughput limit in bytes per second. +I<--write-bytes-sec> specifies write throughput limit in bytes per second. +I<--total-iops-sec> specifies total I/O operations limit per second. +I<--read-iops-sec> specifies read I/O operations limit per second. +I<--write-iops-sec> specifies write I/O operations limit per second. Bytes and iops values are independent, but setting only one value (such -as --read_bytes_sec) resets the other two in that category to unlimited. +as --read-bytes-sec) resets the other two in that category to unlimited. An explicit 0 also clears any limit. A non-zero value for a given total cannot be mixed with non-zero values for read or write. -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Command line interfaces should use dash, not underscore, as many keyboard layouts allow that to be typed with fewer shift key presses.
Also, the US spelling of --tunneled gets more google hits than the UK spelling of --tunnelled.
* tools/virsh.c (opts_migrate): Allow US variant. (opts_blkdeviotune): Prefer - over _. * tools/virsh.pod (blkdeviotune): Fix spelling. ---
ACK Peter

Just because our public API has a typo doesn't mean that virsh has to keep the typo. * tools/virsh.c (VSH_CMD_FLAG_ALIAS): New flag. (nodedevCmds): Use it. (cmdHelp): Omit alias commands. (cmdNodeDeviceDettach): Rename... (cmdNodeDeviceDetach): ...to this. * tools/virsh.pod (nodedev-detach): Document it. --- v2: no real change tools/virsh.c | 30 +++++++++++++++++++----------- tools/virsh.pod | 10 ++++++---- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 75a1a3b..4361a6b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -212,6 +212,7 @@ typedef struct vshCmdOpt { */ enum { VSH_CMD_FLAG_NOCONNECT = (1 << 0), /* no prior connection needed */ + VSH_CMD_FLAG_ALIAS = (1 << 1), /* command is an alias */ }; /* @@ -685,9 +686,12 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, _(" %s (help keyword '%s'):\n"), grp->name, grp->keyword); - for (def = grp->commands; def->name; def++) + for (def = grp->commands; def->name; def++) { + if (def->flags & VSH_CMD_FLAG_ALIAS) + continue; vshPrint(ctl, " %-30s %s\n", def->name, _(vshCmddefGetInfo(def, "help"))); + } vshPrint(ctl, "\n"); } @@ -12939,22 +12943,22 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd) } /* - * "nodedev-dettach" command + * "nodedev-detach" command */ -static const vshCmdInfo info_node_device_dettach[] = { - {"help", N_("dettach node device from its device driver")}, - {"desc", N_("Dettach node device from its device driver before assigning to a domain.")}, +static const vshCmdInfo info_node_device_detach[] = { + {"help", N_("detach node device from its device driver")}, + {"desc", N_("Detach node device from its device driver before assigning to a domain.")}, {NULL, NULL} }; -static const vshCmdOptDef opts_node_device_dettach[] = { +static const vshCmdOptDef opts_node_device_detach[] = { {"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("device key")}, {NULL, 0, 0, NULL} }; static bool -cmdNodeDeviceDettach (vshControl *ctl, const vshCmd *cmd) +cmdNodeDeviceDetach (vshControl *ctl, const vshCmd *cmd) { const char *name = NULL; virNodeDevicePtr device; @@ -12969,10 +12973,12 @@ cmdNodeDeviceDettach (vshControl *ctl, const vshCmd *cmd) return false; } + /* Yes, our public API is misspelled. At least virsh can accept + * either spelling. */ if (virNodeDeviceDettach(device) == 0) { - vshPrint(ctl, _("Device %s dettached\n"), name); + vshPrint(ctl, _("Device %s detached\n"), name); } else { - vshError(ctl, _("Failed to dettach device %s"), name); + vshError(ctl, _("Failed to detach device %s"), name); ret = false; } virNodeDeviceFree(device); @@ -17090,8 +17096,10 @@ static const vshCmdDef nodedevCmds[] = { info_node_device_create, 0}, {"nodedev-destroy", cmdNodeDeviceDestroy, opts_node_device_destroy, info_node_device_destroy, 0}, - {"nodedev-dettach", cmdNodeDeviceDettach, opts_node_device_dettach, - info_node_device_dettach, 0}, + {"nodedev-detach", cmdNodeDeviceDetach, opts_node_device_detach, + info_node_device_detach, 0}, + {"nodedev-dettach", cmdNodeDeviceDetach, opts_node_device_detach, + info_node_device_detach, VSH_CMD_FLAG_ALIAS}, {"nodedev-dumpxml", cmdNodeDeviceDumpXML, opts_node_device_dumpxml, info_node_device_dumpxml, 0}, {"nodedev-list", cmdNodeListDevices, opts_node_list_devices, diff --git a/tools/virsh.pod b/tools/virsh.pod index 1bc55c4..b365624 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1442,7 +1442,7 @@ Attach a device to the domain, using a device definition in an XML file. See the documentation to learn about libvirt XML format for a device. For cdrom and floppy devices, this command only replaces the media within the single existing device; consider using B<update-device> for this usage. -For passthrough host devices, see also B<nodedev-dettach>, needed if +For passthrough host devices, see also B<nodedev-detach>, needed if the device does not use managed mode. =item B<attach-disk> I<domain-id> I<source> I<target> @@ -1571,7 +1571,7 @@ guest domains, nor by multiple active guests at once. If the <hostdev> description includes the attribute B<managed='yes'>, and the hypervisor driver supports it, then the device is in managed mode, and attempts to use that passthrough device in an active guest will -automatically behave as if B<nodedev-dettach> (guest start, device +automatically behave as if B<nodedev-detach> (guest start, device hot-plug) and B<nodedev-reattach> (guest stop, device hot-unplug) were called at the right points (currently, qemu does this for PCI devices, but not USB). If a device is not marked as managed, then it must @@ -1596,11 +1596,13 @@ Destroy (stop) a device on the host. Note that this makes libvirt quit managing a host device, and may even make that device unusable by the rest of the physical host until a reboot. -=item B<nodedev-dettach> I<nodedev> +=item B<nodedev-detach> I<nodedev> Detach I<nodedev> from the host, so that it can safely be used by guests via <hostdev> passthrough. This is reversed with B<nodedev-reattach>, and is done automatically for managed devices. +For compatibility purposes, this command can also be spelled +B<nodedev-dettach>. =item B<nodedev-dumpxml> I<nodedev> @@ -1621,7 +1623,7 @@ formatted in a tree representing parents of each node. Declare that I<nodedev> is no longer in use by any guests, and that the host can resume normal use of the device. This is done automatically for devices in managed mode, but must be done explicitly -to match any explicit B<nodedev-dettach>. +to match any explicit B<nodedev-detach>. =item B<nodedev-reset> I<nodedev> -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Just because our public API has a typo doesn't mean that virsh has to keep the typo.
* tools/virsh.c (VSH_CMD_FLAG_ALIAS): New flag. (nodedevCmds): Use it. (cmdHelp): Omit alias commands. (cmdNodeDeviceDettach): Rename... (cmdNodeDeviceDetach): ...to this. * tools/virsh.pod (nodedev-detach): Document it. ---
Osier ACKed this in his review of the previous posting. Peter

Now can now do: virsh vol-resize $vol 10M virsh blockresize $dom $vol 10M to get both interfaces to resize to 10MiB. The remaining wart is that vol-resize defaults to bytes, but blockresize defaults to KiB, but we can't break existing scripts; oh well, it's no worse than the same wart of the underlying virDomainBlockResize. * tools/virsh.c (vshCommandOptScaledInt): New function. (cmdVolResize): Don't pass negative size. (cmdVolSize): Use new helper routine. (cmdBlockResize): Likewise; also support bytes. * tools/virsh.pod (NOTES): Document suffixes. (blockresize, vol-create-as, vol-resize): Point to notes. --- v2: new tools/virsh.c | 113 +++++++++++++++++++++++++++++++------------------------ tools/virsh.pod | 43 ++++++++++++++++---- 2 files changed, 98 insertions(+), 58 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 4361a6b..d5cc46b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -313,6 +313,10 @@ static int vshCommandOptLongLong(const vshCmd *cmd, const char *name, static int vshCommandOptULongLong(const vshCmd *cmd, const char *name, unsigned long long *value) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; +static int vshCommandOptScaledInt(const vshCmd *cmd, const char *name, + unsigned long long *value, int scale, + unsigned long long max) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; static bool vshCommandOptBool(const vshCmd *cmd, const char *name); static const vshCmdOpt *vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt); @@ -7471,9 +7475,10 @@ static const vshCmdInfo info_block_resize[] = { static const vshCmdOptDef opts_block_resize[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"path", VSH_OT_DATA, VSH_OFLAG_REQ, N_("Fully-qualified path of block device")}, - {"size", VSH_OT_INT, VSH_OFLAG_REQ, N_("New size of the block device in kilobytes, " - "the size must be integer")}, + {"path", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("Fully-qualified path of block device")}, + {"size", VSH_OT_INT, VSH_OFLAG_REQ, + N_("New size of the block device, as scaled integer (default KiB)")}, {NULL, 0, 0, NULL} }; @@ -7494,15 +7499,16 @@ cmdBlockResize(vshControl *ctl, const vshCmd *cmd) return false; } - if (vshCommandOptULongLong(cmd, "size", &size) < 0) { + if (vshCommandOptScaledInt(cmd, "size", &size, 1024, ULLONG_MAX) < 0) { vshError(ctl, "%s", _("Unable to parse integer")); return false; } - if (size > ULLONG_MAX / 1024) { - vshError(ctl, _("Size must be less than %llu"), ULLONG_MAX / 1024); - return false; - } + /* Prefer the older interface of KiB. */ + if (size % 1024 == 0) + size /= 1024; + else + flags |= VIR_DOMAIN_BLOCK_RESIZE_BYTES; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -10892,43 +10898,26 @@ static const vshCmdInfo info_vol_create_as[] = { static const vshCmdOptDef opts_vol_create_as[] = { {"pool", VSH_OT_DATA, VSH_OFLAG_REQ, N_("pool name")}, {"name", VSH_OT_DATA, VSH_OFLAG_REQ, N_("name of the volume")}, - {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, N_("size of the vol with optional k,M,G,T suffix")}, - {"allocation", VSH_OT_STRING, 0, N_("initial allocation size with optional k,M,G,T suffix")}, - {"format", VSH_OT_STRING, 0, N_("file format type raw,bochs,qcow,qcow2,vmdk")}, - {"backing-vol", VSH_OT_STRING, 0, N_("the backing volume if taking a snapshot")}, - {"backing-vol-format", VSH_OT_STRING, 0, N_("format of backing volume if taking a snapshot")}, + {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("size of the vol, as scaled integer (default bytes)")}, + {"allocation", VSH_OT_STRING, 0, + N_("initial allocation size, as scaled integer (default bytes)")}, + {"format", VSH_OT_STRING, 0, + N_("file format type raw,bochs,qcow,qcow2,vmdk")}, + {"backing-vol", VSH_OT_STRING, 0, + N_("the backing volume if taking a snapshot")}, + {"backing-vol-format", VSH_OT_STRING, 0, + N_("format of backing volume if taking a snapshot")}, {NULL, 0, 0, NULL} }; -static int cmdVolSize(const char *data, unsigned long long *val) +static int +cmdVolSize(const char *data, unsigned long long *val) { char *end; if (virStrToLong_ull(data, &end, 10, val) < 0) return -1; - - if (end && *end) { - /* Deliberate fallthrough cases here :-) */ - switch (*end) { - case 'T': - *val *= 1024; - /* fallthrough */ - case 'G': - *val *= 1024; - /* fallthrough */ - case 'M': - *val *= 1024; - /* fallthrough */ - case 'k': - *val *= 1024; - break; - default: - return -1; - } - end++; - if (*end) - return -1; - } - return 0; + return virScaleInteger(val, end, 1, ULLONG_MAX); } static bool @@ -11754,7 +11743,7 @@ static const vshCmdInfo info_vol_resize[] = { static const vshCmdOptDef opts_vol_resize[] = { {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, - N_("new capacity for the vol with optional k,M,G,T suffix")}, + N_("new capacity for the vol, as scaled integer (default bytes)")}, {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, {"allocate", VSH_OT_BOOL, 0, N_("allocate the new capacity, rather than leaving it sparse")}, @@ -11792,16 +11781,12 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, "capacity", &capacityStr) <= 0) goto cleanup; if (delta && *capacityStr == '-') { - if (cmdVolSize(capacityStr + 1, &capacity) < 0) { - vshError(ctl, _("Malformed size %s"), capacityStr); - goto cleanup; - } - capacity = -capacity; - } else { - if (cmdVolSize(capacityStr, &capacity) < 0) { - vshError(ctl, _("Malformed size %s"), capacityStr); - goto cleanup; - } + capacityStr++; + flags |= VIR_STORAGE_VOL_RESIZE_SHRINK; + } + if (cmdVolSize(capacityStr, &capacity) < 0) { + vshError(ctl, _("Malformed size %s"), capacityStr); + goto cleanup; } if (virStorageVolResize(vol, capacity, flags) == 0) { @@ -17851,6 +17836,36 @@ vshCommandOptULongLong(const vshCmd *cmd, const char *name, /** + * vshCommandOptScaledInt: + * @cmd command reference + * @name option name + * @value result + * @scale default of 1 or 1024, if no suffix is present + * @max maximum value permitted + * + * Returns option as long long, scaled according to suffix + * See vshCommandOptInt() + */ +static int +vshCommandOptScaledInt(const vshCmd *cmd, const char *name, + unsigned long long *value, int scale, + unsigned long long max) +{ + const char *str; + int ret; + char *end; + + ret = vshCommandOptString(cmd, name, &str); + if (ret <= 0) + return ret; + if (virStrToLong_ull(str, &end, 10, value) < 0 || + virScaleInteger(value, end, scale, max) < 0) + return -1; + return 1; +} + + +/** * vshCommandOptBool: * @cmd command reference * @name option name diff --git a/tools/virsh.pod b/tools/virsh.pod index b365624..74d3ff5 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -115,6 +115,25 @@ program returned, may not mean the action is complete and you must poll periodically to detect that the guest completed the operation. +Several B<virsh> commands take an optionally scaled integer; if no +scale is provided, then the default is listed in the command (for +historical reasons, some commands default to bytes, while other +commands default to kibibytes). The following case-insensitive +suffixes can be used to select a specfic scale: + b, byte byte 1 + KB kilobyte 1,000 + k, KiB kibibyte 1,024 + MB megabyte 1,000,000 + M, MiB mebibyte 1,048,576 + GB gigabyte 1,000,000,000 + G, GiB gibibyte 1,073,741,824 + TB terabyte 1,000,000,000,000 + T, TiB tebibyte 1,099,511,627,776 + PB petabyte 1,000,000,000,000,000 + P, PiB pebibyte 1,125,899,906,842,624 + EB exabyte 1,000,000,000,000,000,000 + E, EiB exbibyte 1,152,921,504,606,846,976 + =head1 GENERIC COMMANDS The following commands are generic i.e. not specific to a domain. @@ -654,11 +673,14 @@ If I<--info> is specified, the active job information on the specified disk will be printed. I<bandwidth> can be used to set bandwidth limit for the active job. -=item B<blockresize> I<domain> I<--path> I<--size> +=item B<blockresize> I<domain> I<path> I<size> -Resize a block device of domain while the domain is running, I<--path> -specifies the absolute path of the block device, I<--size> specifies the -new size in kilobytes +Resize a block device of domain while the domain is running, I<path> +specifies the absolute path of the block device, I<size> is a scaled +integer (see B<NOTES> above) which defaults to KiB (1024) if there +is no suffix. You must use a suffix of "B" to get bytes (note that +for historical reasons, this differs from B<vol-resize> which defaults +to bytes without a suffix). =item B<dominfo> I<domain-id> @@ -2007,10 +2029,10 @@ Create a volume from a set of arguments. I<pool-or-uuid> is the name or UUID of the storage pool to create the volume in. I<name> is the name of the new volume. -I<capacity> is the size of the volume to be created, with optional k, M, G, or -T suffix. -I<--allocation> I<size> is the initial size to be allocated in the volume, with -optional k, M, G, or T suffix. +I<capacity> is the size of the volume to be created, as a scaled integer +(see B<NOTES> above), defaulting to bytes if there is no suffix. +I<--allocation> I<size> is the initial size to be allocated in the volume, +also as a scaled integer defaulting to bytes. I<--format> I<string> is used in file based storage pools to specify the volume file format to use; raw, bochs, qcow, qcow2, vmdk. I<--backing-vol> I<vol-name-or-key-or-path> is the source backing @@ -2146,7 +2168,10 @@ is in. I<vol-name-or-key-or-path> is the name or key or path of the volume to resize. The new capacity might be sparse unless I<--allocate> is specified. Normally, I<capacity> is the new size, but if I<--delta> is present, then it is added to the existing size. Attempts to shrink -the volume will fail unless I<--shrink> is present. +the volume will fail unless I<--shrink> is present. <capacity> is a +scaled integer (see B<NOTES> above), which defaults to bytes if there +is no suffix. This command is only safe for storage volumes not in +use by an active guest; see also B<blockresize> for live resizing. =back -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
Now can now do:
virsh vol-resize $vol 10M virsh blockresize $dom $vol 10M
to get both interfaces to resize to 10MiB. The remaining wart is that vol-resize defaults to bytes, but blockresize defaults to KiB, but we can't break existing scripts; oh well, it's no worse than the same wart of the underlying virDomainBlockResize.
* tools/virsh.c (vshCommandOptScaledInt): New function. (cmdVolResize): Don't pass negative size. (cmdVolSize): Use new helper routine. (cmdBlockResize): Likewise; also support bytes. * tools/virsh.pod (NOTES): Document suffixes. (blockresize, vol-create-as, vol-resize): Point to notes. ---
static bool @@ -11754,7 +11743,7 @@ static const vshCmdInfo info_vol_resize[] = { static const vshCmdOptDef opts_vol_resize[] = { {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, - N_("new capacity for the vol with optional k,M,G,T suffix")}, + N_("new capacity for the vol, as scaled integer (default bytes)")}, {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, {"allocate", VSH_OT_BOOL, 0, N_("allocate the new capacity, rather than leaving it sparse")}, @@ -11792,16 +11781,12 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, "capacity",&capacityStr)<= 0) goto cleanup; if (delta&& *capacityStr == '-') { - if (cmdVolSize(capacityStr + 1,&capacity)< 0) { - vshError(ctl, _("Malformed size %s"), capacityStr); - goto cleanup; - } - capacity = -capacity; - } else { - if (cmdVolSize(capacityStr,&capacity)< 0) { - vshError(ctl, _("Malformed size %s"), capacityStr); - goto cleanup; - } + capacityStr++; This shift in the string discards the minus sign in error/success messages, but their meaning remains correct. + flags |= VIR_STORAGE_VOL_RESIZE_SHRINK;
This change in code ...
+ } + if (cmdVolSize(capacityStr,&capacity)< 0) { + vshError(ctl, _("Malformed size %s"), capacityStr); + goto cleanup; }
if (virStorageVolResize(vol, capacity, flags) == 0) { @@ -2146,7 +2168,10 @@ is in. I<vol-name-or-key-or-path> is the name or key or path of the volume to resize. The new capacity might be sparse unless I<--allocate> is specified. Normally, I<capacity> is the new size, but if I<--delta> is present, then it is added to the existing size. Attempts to shrink -the volume will fail unless I<--shrink> is present. +the volume will fail unless I<--shrink> is present.<capacity> is
... contradicts with this statment in the virsh man page. With your change, if the delta is negative the shrink flag for the api call is set automaticaly. This was probably added as a safety measure and I'd prefer if we would require the --shrink flag to mark that the user is sure of what he's doing, although it should be obvious enough from the minus sign. (I honestly would prefer a docs change but I'm afraid someone could get mad at us if he "accidentaly" corrupts his images.)
+scaled integer (see B<NOTES> above), which defaults to bytes if there +is no suffix. This command is only safe for storage volumes not in +use by an active guest; see also B<blockresize> for live resizing.
=back
ACK, if you add the check for --shrink. Peter

On 03/07/2012 05:45 AM, Peter Krempa wrote:
On 03/06/2012 01:34 AM, Eric Blake wrote:
Now can now do:
virsh vol-resize $vol 10M virsh blockresize $dom $vol 10M
to get both interfaces to resize to 10MiB. The remaining wart is that vol-resize defaults to bytes, but blockresize defaults to KiB, but we can't break existing scripts; oh well, it's no worse than the same wart of the underlying virDomainBlockResize.
- capacity = -capacity; - } else { - if (cmdVolSize(capacityStr,&capacity)< 0) { - vshError(ctl, _("Malformed size %s"), capacityStr); - goto cleanup; - } + capacityStr++; This shift in the string discards the minus sign in error/success messages, but their meaning remains correct.
The old code was wrong for negating things - the public API is documented as always requiring a non-negative value (the combination of delta and shrink implies a reduction, no negative needed). So this is actually a bug fix.
This was probably added as a safety measure and I'd prefer if we would require the --shrink flag to mark that the user is sure of what he's doing, although it should be obvious enough from the minus sign. (I honestly would prefer a docs change but I'm afraid someone could get mad at us if he "accidentaly" corrupts his images.)
My latest version now requires both --delta and --shrink to be present before ignoring a negative sign.
ACK, if you add the check for --shrink.
Peter
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

The last vestige of the inaccurate 'kilobytes' when we meant 1024 is now gone. And virsh is now useful for setting memory in units other than KiB. * tools/virsh.c (cmdSetmem, cmdSetmaxmem): Use new helper routine, allow passing bogus arguments on to hypervisor to test driver sanity checking, and fix leak on parse error. (cmdMemtuneGetSize): New helper. (cmdMemtune): Use it. * tools/virsh.pod (setmem, setmaxmem, memtune): Document this. --- v2: new tools/virsh.c | 110 +++++++++++++++++++++++++++++++++---------------------- tools/virsh.pod | 48 ++++++++++++------------ 2 files changed, 90 insertions(+), 68 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d5cc46b..b93706f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5668,7 +5668,7 @@ cleanup: } /* - * "setmemory" command + * "setmem" command */ static const vshCmdInfo info_setmem[] = { {"help", N_("change memory allocation")}, @@ -5678,7 +5678,9 @@ static const vshCmdInfo info_setmem[] = { static const vshCmdOptDef opts_setmem[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("number of kilobytes of memory")}, + {"kilobytes", VSH_OT_ALIAS, 0, "size"}, + {"size", VSH_OT_INT, VSH_OFLAG_REQ, + N_("new memory size, as scaled integer (default KiB)")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, @@ -5689,8 +5691,9 @@ static bool cmdSetmem(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - virDomainInfo info; - unsigned long kilobytes = 0; + unsigned long long bytes = 0; + unsigned long long max; + unsigned long kibibytes = 0; bool ret = true; int config = vshCommandOptBool(cmd, "config"); int live = vshCommandOptBool(cmd, "live"); @@ -5719,36 +5722,25 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptUL(cmd, "kilobytes", &kilobytes) < 0) { + /* The API expects 'unsigned long' KiB, so depending on whether we + * are 32-bit or 64-bit determines the maximum we can use. */ + if (sizeof(kibibytes) < sizeof(max)) + max = 1024ull * ULONG_MAX; + else + max = ULONG_MAX; + if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) { vshError(ctl, "%s", _("memory size has to be a number")); - return false; - } - - if (kilobytes <= 0) { - virDomainFree(dom); - vshError(ctl, _("Invalid value of %lu for memory size"), kilobytes); - return false; - } - - if (virDomainGetInfo(dom, &info) != 0) { virDomainFree(dom); - vshError(ctl, "%s", _("Unable to verify MaxMemorySize")); - return false; - } - - if (kilobytes > info.maxMem) { - virDomainFree(dom); - vshError(ctl, _("Requested memory size %lu kb is larger than maximum of %lu kb"), - kilobytes, info.maxMem); return false; } + kibibytes = VIR_DIV_UP(bytes, 1024); if (flags == -1) { - if (virDomainSetMemory(dom, kilobytes) != 0) { + if (virDomainSetMemory(dom, kibibytes) != 0) { ret = false; } } else { - if (virDomainSetMemoryFlags(dom, kilobytes, flags) < 0) { + if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) { ret = false; } } @@ -5768,7 +5760,9 @@ static const vshCmdInfo info_setmaxmem[] = { static const vshCmdOptDef opts_setmaxmem[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum memory limit in kilobytes")}, + {"kilobytes", VSH_OT_ALIAS, 0, "size"}, + {"size", VSH_OT_INT, VSH_OFLAG_REQ, + N_("new maximum memory size, as scaled integer (default KiB)")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, @@ -5779,7 +5773,9 @@ static bool cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - int kilobytes = 0; + unsigned long long bytes = 0; + unsigned long long max; + unsigned long kibibytes = 0; bool ret = true; int config = vshCommandOptBool(cmd, "config"); int live = vshCommandOptBool(cmd, "live"); @@ -5807,24 +5803,26 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptInt(cmd, "kilobytes", &kilobytes) < 0) { + /* The API expects 'unsigned long' KiB, so depending on whether we + * are 32-bit or 64-bit determines the maximum we can use. */ + if (sizeof(kibibytes) < sizeof(max)) + max = 1024ull * ULONG_MAX; + else + max = ULONG_MAX; + if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) { vshError(ctl, "%s", _("memory size has to be a number")); - return false; - } - - if (kilobytes <= 0) { virDomainFree(dom); - vshError(ctl, _("Invalid value of %d for memory size"), kilobytes); return false; } + kibibytes = VIR_DIV_UP(bytes, 1024); if (flags == -1) { - if (virDomainSetMaxMemory(dom, kilobytes) != 0) { + if (virDomainSetMaxMemory(dom, kibibytes) != 0) { vshError(ctl, "%s", _("Unable to change MaxMemorySize")); ret = false; } } else { - if (virDomainSetMemoryFlags(dom, kilobytes, flags) < 0) { + if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) { vshError(ctl, "%s", _("Unable to change MaxMemorySize")); ret = false; } @@ -5997,21 +5995,45 @@ static const vshCmdInfo info_memtune[] = { static const vshCmdOptDef opts_memtune[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"hard-limit", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Max memory in kilobytes")}, + N_("Max memory, as scaled integer (default KiB)")}, {"soft-limit", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Memory during contention in kilobytes")}, + N_("Memory during contention, as scaled integer (default KiB)")}, {"swap-hard-limit", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Max memory plus swap in kilobytes")}, + N_("Max memory plus swap, as scaled integer (default KiB)")}, {"min-guarantee", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Min guaranteed memory in kilobytes")}, + N_("Min guaranteed memory, as scaled integer (default KiB)")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, {NULL, 0, 0, NULL} }; +static int +cmdMemtuneGetSize(const vshCmd *cmd, const char *name, long long *value) +{ + int ret; + unsigned long long tmp; + const char *str; + char *end; + + ret = vshCommandOptString(cmd, name, &str); + if (ret <= 0) + return ret; + if (virStrToLong_ll(str, &end, 10, value) < 0) + return -1; + if (*value < 0) { + *value = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + return 1; + } + tmp = *value; + if (virScaleInteger(&tmp, end, 1024, LLONG_MAX) < 0) + return -1; + *value = VIR_DIV_UP(tmp, 1024); + return 0; +} + static bool -cmdMemtune(vshControl * ctl, const vshCmd * cmd) +cmdMemtune(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; long long hard_limit = 0, soft_limit = 0, swap_hard_limit = 0; @@ -6044,10 +6066,10 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptLongLong(cmd, "hard-limit", &hard_limit) < 0 || - vshCommandOptLongLong(cmd, "soft-limit", &soft_limit) < 0 || - vshCommandOptLongLong(cmd, "swap-hard-limit", &swap_hard_limit) < 0 || - vshCommandOptLongLong(cmd, "min-guarantee", &min_guarantee) < 0) { + if (cmdMemtuneGetSize(cmd, "hard-limit", &hard_limit) < 0 || + cmdMemtuneGetSize(cmd, "soft-limit", &soft_limit) < 0 || + cmdMemtuneGetSize(cmd, "swap-hard-limit", &swap_hard_limit) < 0 || + cmdMemtuneGetSize(cmd, "min-guarantee", &min_guarantee) < 0) { vshError(ctl, "%s", _("Unable to parse integer parameter")); goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index 74d3ff5..a13a3b2 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1153,7 +1153,7 @@ B<Examples> # send a tab, held for 1 second virsh send-key --holdtime 1000 0xf -=item B<setmem> I<domain-id> B<kilobytes> [[I<--config>] [I<--live>] | +=item B<setmem> I<domain-id> B<size> [[I<--config>] [I<--live>] | [I<--current>]] Change the memory allocation for a guest domain. @@ -1164,15 +1164,17 @@ Both I<--live> and I<--config> flags may be given, but I<--current> is exclusive. If no flag is specified, behavior is different depending on hypervisor. -Some hypervisors require a larger granularity than kilobytes, and requests +I<size> is a scaled integer (see B<NOTES> above); it defaults to kibibytes +(1024) unless you provide a suffix (and the older option name I<--kilobytes> +is available as a deprecated synonym) . Libvirt rounds up to the nearest +kibibyte. Some hypervisors require a larger granularity than KiB, and requests that are not an even multiple will be rounded up. For example, vSphere/ESX -rounds the parameter up unless the kB argument is evenly divisible by 1024 -(that is, the kB argument happens to represent megabytes). +rounds the parameter up to mebibytes (1024 kibibytes). For Xen, you can only adjust the memory of a running domain if the domain is paravirtualized or running the PV balloon driver. -=item B<setmaxmem> I<domain-id> B<kilobytes> [[I<--config>] [I<--live>] | +=item B<setmaxmem> I<domain-id> B<size> [[I<--config>] [I<--live>] | [I<--current>]] Change the maximum memory allocation limit for a guest domain. @@ -1185,22 +1187,24 @@ on hypervisor. This command works for at least the Xen, QEMU/KVM and vSphere/ESX hypervisors. -Some hypervisors require a larger granularity than kilobytes, rounding up -requests that are not an even multiple of the desired amount. vSphere/ESX -is one of these, requiring the parameter to be evenly divisible by 4MB. For -vSphere/ESX, 263168 (257MB) would be rounded up because it's not a multiple -of 4MB, while 266240 (260MB) is valid without rounding. - +I<size> is a scaled integer (see B<NOTES> above); it defaults to kibibytes +(1024) unless you provide a suffix (and the older option name I<--kilobytes> +is available as a deprecated synonym) . Libvirt rounds up to the nearest +kibibyte. Some hypervisors require a larger granularity than KiB, and requests +that are not an even multiple will be rounded up. For example, vSphere/ESX +rounds the parameter up to mebibytes (1024 kibibytes). -=item B<memtune> I<domain-id> [I<--hard-limit> B<kilobytes>] -[I<--soft-limit> B<kilobytes>] [I<--swap-hard-limit> B<kilobytes>] -[I<--min-guarantee> B<kilobytes>] [[I<--config>] [I<--live>] | [I<--current>]] +=item B<memtune> I<domain-id> [I<--hard-limit> B<size>] +[I<--soft-limit> B<size>] [I<--swap-hard-limit> B<size>] +[I<--min-guarantee> B<size>] [[I<--config>] [I<--live>] | [I<--current>]] Allows you to display or set the domain memory parameters. Without flags, the current settings are displayed; with a flag, the appropriate limit is adjusted if supported by the hypervisor. LXC and QEMU/KVM support I<--hard-limit>, I<--soft-limit>, and I<--swap-hard-limit>. -I<--min-guarantee> is supported only by ESX hypervisor. +I<--min-guarantee> is supported only by ESX hypervisor. Each of these +limits are scaled integers (see B<NOTES> above), with a default of +kibibytes (blocks of 1024) if no suffix is present. If I<--live> is specified, affect a running guest. If I<--config> is specified, affect the next boot of a persistent guest. @@ -1218,24 +1222,20 @@ one needs guess and try. =item I<--hard-limit> -The maximum memory the guest can use. The units for this value are kilobytes -(i.e. blocks of 1024 bytes). +The maximum memory the guest can use. =item I<--soft-limit> -The memory limit to enforce during memory contention. The units for this -value are kilobytes (i.e. blocks of 1024 bytes). +The memory limit to enforce during memory contention. =item I<--swap-hard-limit> -The maximum memory plus swap the guest can use. The units for this value are -kilobytes (i.e. blocks of 1024 bytes). This has to be more than hard-limit -value provided. +The maximum memory plus swap the guest can use. This has to be more +than hard-limit value provided. =item I<--min-guarantee> -The guaranteed minimum memory allocation for the guest. The units for this -value are kilobytes (i.e. blocks of 1024 bytes). +The guaranteed minimum memory allocation for the guest. =back -- 1.7.7.6

On 03/06/2012 01:34 AM, Eric Blake wrote:
The last vestige of the inaccurate 'kilobytes' when we meant 1024 is now gone. And virsh is now useful for setting memory in units other than KiB.
* tools/virsh.c (cmdSetmem, cmdSetmaxmem): Use new helper routine, allow passing bogus arguments on to hypervisor to test driver sanity checking, and fix leak on parse error. (cmdMemtuneGetSize): New helper. (cmdMemtune): Use it. * tools/virsh.pod (setmem, setmaxmem, memtune): Document this. ---
static const vshCmdOptDef opts_memtune[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"hard-limit", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Max memory in kilobytes")}, + N_("Max memory, as scaled integer (default KiB)")}, {"soft-limit", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Memory during contention in kilobytes")}, + N_("Memory during contention, as scaled integer (default KiB)")}, {"swap-hard-limit", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Max memory plus swap in kilobytes")}, + N_("Max memory plus swap, as scaled integer (default KiB)")}, {"min-guarantee", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Min guaranteed memory in kilobytes")}, + N_("Min guaranteed memory, as scaled integer (default KiB)")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, {NULL, 0, 0, NULL} };
+static int +cmdMemtuneGetSize(const vshCmd *cmd, const char *name, long long *value)
As this is a helper function rename it please to vshMemtuneGetSize to avoid confusion with command functions.
+{ + int ret; + unsigned long long tmp; + const char *str; + char *end; + + ret = vshCommandOptString(cmd, name,&str); + if (ret<= 0) + return ret; + if (virStrToLong_ll(str,&end, 10, value)< 0) + return -1; + if (*value< 0) { + *value = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + return 1; + } + tmp = *value; + if (virScaleInteger(&tmp, end, 1024, LLONG_MAX)< 0) + return -1; + *value = VIR_DIV_UP(tmp, 1024); + return 0; +} + static bool -cmdMemtune(vshControl * ctl, const vshCmd * cmd) +cmdMemtune(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; long long hard_limit = 0, soft_limit = 0, swap_hard_limit = 0;
ACK with the name change. Peter

On 03/07/2012 06:14 AM, Peter Krempa wrote:
On 03/06/2012 01:34 AM, Eric Blake wrote:
The last vestige of the inaccurate 'kilobytes' when we meant 1024 is now gone. And virsh is now useful for setting memory in units other than KiB.
+static int +cmdMemtuneGetSize(const vshCmd *cmd, const char *name, long long *value)
As this is a helper function rename it please to vshMemtuneGetSize to avoid confusion with command functions.
Sure.
ACK with the name change.
Thanks for the review. Nits fixed, and series pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Christophe Fergeau
-
Eric Blake
-
Peter Krempa