[virt-tools-list] [PATCH v3 1/4] virtinst: Add vsock device type

[Slavomir Kaslev]

On 12/18/18 3:59 PM, Marc Hartmayer wrote:

> On Fri, Dec 14, 2018 at 03:34 PM +0100, Slavomir Kaslev <kaslevs at vmware.com> wrote:
>> VSOCK sockets allow communication between virtual machines and the host they are
>> running on.
>>
>> This patch adds vsock device support along with clitest for the new properties.
>>
>> Signed-off-by: Slavomir Kaslev <kaslevs at vmware.com>
>> ---
> […snip…]
>
>>
>>   def add_guest_xml_options(geng):
>> @@ -2577,6 +2581,23 @@ ParserPanic.add_arg(None, "model", cb=ParserPanic.set_model_cb,
>>   ParserPanic.add_arg("iobase", "iobase")
>>
>>
>> +###################
>> +# --vsock parsing #
>> +###################
>> +
> I know this blank line is everywhere in this module… but why? It
> violates against pep8…
>
> Wouldn’t it make more sense to have an docstring for the class instead?

It makes perfect sense to me.

Since I'm not very familiar with the coding style this project uses, I 
was trying to follow the style of the surrounding code (e.g. 
https://github.com/virt-manager/virt-manager/blob/b91393e6c35b0e2903dbb50bb57a64464a7a3802/virtinst/devices/panic.py#L48 
or 
https://github.com/virt-manager/virt-manager/blob/b91393e6c35b0e2903dbb50bb57a64464a7a3802/virtinst/cli.py#L1298 
) but I don't have any strong feelings either way.

Though, if a decision is made to switch to docstrings instead, it would 
better be done in bulk (semi-automated?) over the whole project in a 
separate patch set rather than start from here.

--Slavi

>
>> +class ParserVsock(VirtCLIParser):
>> +    cli_arg_name = "vsock"
>> +    propname = "devices.vsock"
>> +    remove_first = "model"
>> +    stub_none = False
>> +
>> +_register_virt_parser(ParserVsock)
>> +_add_device_address_args(ParserVsock)
>> +ParserVsock.add_arg("model", "model")
>> +ParserVsock.add_arg("auto_cid", "auto_cid")
>> +ParserVsock.add_arg("cid", "cid")
>> +
>> +
>>   ######################################################
>>   # --serial, --parallel, --channel, --console parsing #
>>   ######################################################
>> diff --git a/virtinst/devices/__init__.py b/virtinst/devices/__init__.py
>> index 6da0766d..6120f5d0 100644
>> --- a/virtinst/devices/__init__.py
>> +++ b/virtinst/devices/__init__.py
>> @@ -22,6 +22,7 @@ from .redirdev import DeviceRedirdev
>>   from .rng import DeviceRng
>>   from .tpm import DeviceTpm
>>   from .video import DeviceVideo
>> +from .vsock import DeviceVsock
>>   from .watchdog import DeviceWatchdog
>>
>>
>> diff --git a/virtinst/devices/vsock.py b/virtinst/devices/vsock.py
>> new file mode 100644
>> index 00000000..27b0cc3d
>> --- /dev/null
>> +++ b/virtinst/devices/vsock.py
>> @@ -0,0 +1,42 @@
>> +# Copyright (C) 2018 VMware, Inc.
>> +#
>> +# Copyright 2018
>> +# Slavomir Kaslev <kaslevs at vmware.com>
>> +#
>> +# This work is licensed under the GNU GPLv2 or later.
>> +# See the COPYING file in the top-level directory.
>> +
>> +from .device import Device
>> +from ..xmlbuilder import XMLProperty
>> +
>> +
>> +class DeviceVsock(Device):
>> +    XML_NAME = "vsock"
>> +
>> +    model = XMLProperty("./@model")
>> +    auto_cid = XMLProperty("./cid/@auto", is_yesno=True)
>> +    cid = XMLProperty("./cid/@address", is_int=True)
>> +
>> +    MIN_GUEST_CID = 3
>> +
>> +
>> +    ##############
>> +    # Validation #
>> +    ##############
>> +
>> +    def validate(self):
> The same goes here :)
>
>> +        if not self.auto_cid and (self.cid is None or
>> +                                  self.cid < self.MIN_GUEST_CID):
>> +            raise ValueError(_("guest CID {0} must be >= 3").format(self.cid))
>> +
>> +
>> +    ##################
>> +    # Default config #
>> +    ##################
>> +
>> +    def set_defaults(self, guest):
> And here.
>
>> +        if not self.model:
>> +            self.model = "virtio"
>> +
>> +        if self.auto_cid is None and self.cid is None:
>> +            self.auto_cid = True
>> diff --git a/virtinst/guest.py b/virtinst/guest.py
>> index eeb40cb6..9acff3b9 100644
>> --- a/virtinst/guest.py
>> +++ b/virtinst/guest.py
>> @@ -29,7 +29,7 @@ class _DomainDevices(XMLBuilder):
>>               'smartcard', 'serial', 'parallel', 'console', 'channel',
>>               'input', 'tpm', 'graphics', 'sound', 'video', 'hostdev',
>>               'redirdev', 'watchdog', 'memballoon', 'rng', 'panic',
>> -            'memory']
>> +            'memory', 'vsock']
>>
>>
>>       disk = XMLChildProperty(DeviceDisk)
>> @@ -53,6 +53,7 @@ class _DomainDevices(XMLBuilder):
>>       rng = XMLChildProperty(DeviceRng)
>>       panic = XMLChildProperty(DevicePanic)
>>       memory = XMLChildProperty(DeviceMemory)
>> +    vsock = XMLChildProperty(DeviceVsock)
>>
>>       def get_all(self):
>>           retlist = []
>> --
>> 2.19.1
>>
>> _______________________________________________
>> virt-tools-list mailing list
>> virt-tools-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/virt-tools-list
> Kind regards / Beste Grüße
>     Marc Hartmayer
>
> IBM Deutschland Research & Development GmbH
> Vorsitzende des Aufsichtsrats: Matthias Hartmann
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
>
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list