Standardize vnc fields in virt instance update

truenas · Jan 13, 2025 · 4e19c6a · 4e19c6a
1 parent 10286b8
commit 4e19c6a
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 12 deletions.
diff --git a/src/middlewared/middlewared/api/v25_04_0/virt_instance.py b/src/middlewared/middlewared/api/v25_04_0/virt_instance.py
@@ -114,6 +114,7 @@ class VirtInstanceUpdate(BaseModel, metaclass=ForUpdateMetaclass):
     cpu: str | None = None
     memory: MemoryType | None = None
     vnc_port: int | None = Field(ge=5900, le=65535)
+    enable_vnc: bool
 
 
 class VirtInstanceUpdateArgs(BaseModel):

diff --git a/src/middlewared/middlewared/plugins/virt/instance.py b/src/middlewared/middlewared/plugins/virt/instance.py
@@ -156,14 +156,11 @@ async def validate(self, new, schema_name, verrors, old=None):
                 verrors.add(f'{schema_name}.cpu', 'Cannot reserve more than system cores')
 
         if old:
-            if new.get('vnc_port'):
-                # If in update case, user specifies a vnc port, we automatically assume he wants to enable vnc
-                # this makes it easier to change existing vnc port and set it as well
-                new['enable_vnc'] = True
-            elif 'vnc_port' in new and new['vnc_port'] is None:
-                # This is the case to handle when we want to disable VNC
-                new['enable_vnc'] = False
-            elif 'vnc_port' not in new and old['vnc_enabled'] and old['vnc_port']:
+            if new.get('enable_vnc') is False:
+                new['vnc_port'] = None
+            elif 'enable_vnc' not in new and new.get('vnc_port'):
+                verrors.add(f'{schema_name}.enable_vnc', 'Should be set when vnc_port is specified')
+            elif 'enable_vnc' not in new and old['vnc_enabled'] and old['vnc_port']:
                 # We want to handle the case where nothing has been changed on vnc attrs
                 new.update({
                     'enable_vnc': True,
@@ -213,7 +210,7 @@ def __data_to_config(self, data: dict, raw: dict = None, instance_type=None):
                 config['user.ix_old_raw_qemu_config'] = raw.get('raw.qemu', '') if raw else ''
                 config['raw.qemu'] = f'-vnc :{data["vnc_port"] - VNC_BASE_PORT}'
             if data.get('enable_vnc') is False:
-                config['user.ix_old_raw_qemu_config'] = raw['raw.qemu'] if raw else ''
+                config['user.ix_old_raw_qemu_config'] = raw.get('raw.qemu', '') if raw else ''
                 config['raw.qemu'] = ''
 
         return config

diff --git a/tests/api2/test_virt_vm.py b/tests/api2/test_virt_vm.py
@@ -121,14 +121,14 @@ def test_vm_props(vm):
     assert instance['vnc_port'] == VNC_PORT, instance
 
     # Going to unset VNC
-    call('virt.instance.update', VM_NAME, {'vnc_port': None}, job=True)
+    call('virt.instance.update', VM_NAME, {'enable_vnc': False}, job=True)
     instance = call('virt.instance.get_instance', VM_NAME, {'extra': {'raw': True}})
     assert instance['raw']['config']['user.ix_old_raw_qemu_config'] == f'-vnc :{VNC_PORT - 5900}'
     assert instance['vnc_enabled'] is False, instance
     assert instance['vnc_port'] is None, instance
 
     # Going to update port
-    call('virt.instance.update', VM_NAME, {'vnc_port': 6901}, job=True)
+    call('virt.instance.update', VM_NAME, {'vnc_port': 6901, 'enable_vnc': True}, job=True)
     instance = call('virt.instance.get_instance', VM_NAME, {'extra': {'raw': True}})
     assert instance['raw']['config'].get('user.ix_old_raw_qemu_config') is None
     assert instance['raw']['config']['raw.qemu'] == f'-vnc :{1001}'
@@ -162,7 +162,7 @@ def test_vnc_validation_on_vm_create(virt_pool, enable_vnc, vnc_port, error_msg)
              'source_type': None,
              'vnc_port': vnc_port,
              'enable_vnc': enable_vnc,
-         }, job=True)
+        }, job=True)
 
     assert ve.value.errors[0].errmsg == error_msg