[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#27887] [PATCH] services: Add libvirt services
From: |
Ryan Moe |
Subject: |
[bug#27887] [PATCH] services: Add libvirt services |
Date: |
Mon, 21 Aug 2017 19:38:25 -0700 |
I did some more testing and everything looks good to me. I've
addressed your previous comments as well (see below). I wasn't sure if
I needed to squash the commits so I just included all three patches. I
did reorder them to place your wrap patch before mine since the
libvirt service depends on that change.
>> +(define libvirt-service-type
>> + (service-type (name 'libvirt)
>> + (extensions
>> + (list
>> + (service-extension polkit-service-type (compose list
>> libvirt-configuration-libvirt))
>
> Line length could be better here, just by putting the (compose ... )
> bit on the line after the polkit-service-type.
>
Agreed. This is fixed in the attached patch.
>> + (service-extension profile-service-type
>> + (compose list
>> + (lambda (package) qemu)
>> + libvirt-configuration-libvirt))
>
> This confused me for a bit, until I realised that a simpler way of
> expressing this would be (const (list qemu)) if I'm correct? Also, it
> would be good to explain why this needs to happen in a comment.
>
That didn't work. qemu doesn't need to be installed in the system
profile anyway so I've removed this.
>> +(define virtlog-service-type
>> + (service-type (name 'virtlogd)
>> + (extensions
>> + (list
>> + (service-extension profile-service-type
>> + (compose list
>> +
>> virtlog-configuration-libvirt))
>
> What function does this extension have? As far as I understood from the
> documentation you wrote, this is used from the libvirt service.
Now that I have a better understanding of how this works I realize
this was unnecessary and it's been removed too.
0001-packages-virtualization-Wrap-libvirtd-with-iproute-a.patch
Description: Text Data
0002-services-Add-libvirt-services.patch
Description: Text Data
0003-tests-Add-libvirt-service-type-test.patch
Description: Text Data
- [bug#27887] [PATCH] services: Add libvirt services, Christopher Baines, 2017/08/18
- [bug#27887] [PATCH] services: Add libvirt services, Christopher Baines, 2017/08/18
- [bug#27887] [PATCH] services: Add libvirt services, Ryan Moe, 2017/08/18
- [bug#27887] [PATCH] services: Add libvirt services, Christopher Baines, 2017/08/19
- [bug#27887] [PATCH] services: Add libvirt services, Ryan Moe, 2017/08/19
- [bug#27887] [PATCH] services: Add libvirt services, Christopher Baines, 2017/08/19
- [bug#27887] [PATCH] services: Add libvirt services, Ryan Moe, 2017/08/21
- [bug#27887] [PATCH] services: Add libvirt services,
Ryan Moe <=
- [bug#27887] [PATCH] services: Add libvirt services, Christopher Baines, 2017/08/22
- bug#27887: [PATCH] services: Add libvirt services, Christopher Baines, 2017/08/23