virt-install: Share required option logic for resources

There's some cases we were incorrectly setting osinfo defaults, when
mem or storage values had already been specified elsewhere
This commit is contained in:
Cole Robinson 2019-06-10 19:50:14 -04:00
parent 66fe00ddee
commit 01a07a1051
3 changed files with 19 additions and 23 deletions

View File

@ -90,7 +90,6 @@
<type arch="x86_64" machine="q35">hvm</type> <type arch="x86_64" machine="q35">hvm</type>
<loader>/foo/bar</loader> <loader>/foo/bar</loader>
<boot dev="network"/> <boot dev="network"/>
<boot dev="hd"/>
<smbios mode="sysinfo"/> <smbios mode="sysinfo"/>
<bootmenu enable="no"/> <bootmenu enable="no"/>
<bios rebootTimeout="3"/> <bios rebootTimeout="3"/>
@ -160,11 +159,6 @@
</pm> </pm>
<devices> <devices>
<emulator>/new/emu</emulator> <emulator>/new/emu</emulator>
<disk type="file" device="disk">
<driver name="qemu" type="qcow2"/>
<source file="/var/lib/libvirt/images/foobar.qcow2"/>
<target dev="vda" bus="virtio"/>
</disk>
<controller type="usb" index="0" model="qemu-xhci" ports="15"/> <controller type="usb" index="0" model="qemu-xhci" ports="15"/>
<controller type="scsi" index="0" model="virtio-scsi"/> <controller type="scsi" index="0" model="virtio-scsi"/>
<filesystem type="mount" accessmode="mapped"> <filesystem type="mount" accessmode="mapped">
@ -382,11 +376,6 @@
</pm> </pm>
<devices> <devices>
<emulator>/new/emu</emulator> <emulator>/new/emu</emulator>
<disk type="file" device="disk">
<driver name="qemu" type="qcow2"/>
<source file="/var/lib/libvirt/images/foobar.qcow2"/>
<target dev="vda" bus="virtio"/>
</disk>
<controller type="usb" index="0" model="qemu-xhci" ports="15"/> <controller type="usb" index="0" model="qemu-xhci" ports="15"/>
<controller type="scsi" index="0" model="virtio-scsi"/> <controller type="scsi" index="0" model="virtio-scsi"/>
<filesystem type="mount" accessmode="mapped"> <filesystem type="mount" accessmode="mapped">

View File

@ -7,8 +7,6 @@
</libosinfo:libosinfo> </libosinfo:libosinfo>
</metadata> </metadata>
<maxMemory slots="2">2097152</maxMemory> <maxMemory slots="2">2097152</maxMemory>
<memory>2097152</memory>
<currentMemory>2097152</currentMemory>
<memoryBacking> <memoryBacking>
<hugepages/> <hugepages/>
<access mode="shared"/> <access mode="shared"/>

View File

@ -306,6 +306,16 @@ def do_test_media_detection(conn, options):
# General option validation # # General option validation #
############################# #############################
def storage_specified(options, guest):
if guest.os.is_container():
return True
return options.disk or options.filesystem
def memory_specified(guest):
return guest.memory or guest.currentMemory or guest.cpu.cells
def validate_required_options(options, guest, installer): def validate_required_options(options, guest, installer):
# Required config. Don't error right away if nothing is specified, # Required config. Don't error right away if nothing is specified,
# aggregate the errors to help first time users get it right # aggregate the errors to help first time users get it right
@ -314,11 +324,10 @@ def validate_required_options(options, guest, installer):
if not guest.name: if not guest.name:
msg += "\n" + _("--name is required") msg += "\n" + _("--name is required")
if not guest.memory and not guest.cpu.cells: if not memory_specified(guest):
msg += "\n" + _("--memory amount in MiB is required") msg += "\n" + _("--memory amount in MiB is required")
if (not guest.os.is_container() and if not storage_specified(options, guest):
not (options.disk or options.filesystem)):
msg += "\n" + ( msg += "\n" + (
_("--disk storage must be specified (override with --disk none)")) _("--disk storage must be specified (override with --disk none)"))
@ -520,21 +529,21 @@ def set_resources_from_osinfo(options, guest):
if guest.os.is_container(): if guest.os.is_container():
return return
# We need to do this upfront, so we don't incorrectly set guest.vcpus
guest.sync_vcpus_topology()
res = guest.osinfo.get_recommended_resources() res = guest.osinfo.get_recommended_resources()
storage = res.get_recommended_storage(guest.os.arch) storage = res.get_recommended_storage(guest.os.arch)
ram = res.get_recommended_ram(guest.os.arch) ram = res.get_recommended_ram(guest.os.arch)
ncpus = res.get_recommended_ncpus(guest.os.arch) ncpus = res.get_recommended_ncpus(guest.os.arch)
if ram and not guest.currentMemory: if ram and not memory_specified(guest):
guest.currentMemory = ram // 1024 guest.currentMemory = ram // 1024
if ncpus and not guest.vcpus: if ncpus:
guest.vcpus = ncpus # We need to do this upfront, so we don't incorrectly set guest.vcpus
guest.sync_vcpus_topology()
if not guest.vcpus:
guest.vcpus = ncpus
if storage and not options.disk: if storage and not storage_specified(options, guest):
diskstr = 'size=%d' % (storage // (1024 ** 3)) diskstr = 'size=%d' % (storage // (1024 ** 3))
logging.debug("Generated default libosinfo '--disk %s'", diskstr) logging.debug("Generated default libosinfo '--disk %s'", diskstr)
options.disk = [diskstr] options.disk = [diskstr]