- Devel - libvirt List Archives

[libvirt] [PATCH] Fix security driver handling of FIFOs with QEMU
by Daniel P. Berrange 03 May '11

03 May '11

When setting up a FIFO for QEMU, it allows either a pair of fifos used unidirectionally, or a single fifo used bidirectionally. Look for the bidirectional fifo first when labelling since that is more useful * src/security/security_dac.c, src/security/security_selinux.c: Fix fifo handling --- src/security/security_dac.c | 19 ++++++++++++------- src/security/security_selinux.c | 19 ++++++++++++------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index fba2d1d..b8642d2 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -406,14 +406,19 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, break; case VIR_DOMAIN_CHR_TYPE_PIPE: - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { - virReportOOMError(); - goto done; + if (virFileExists(dev->data.file.path)) { + if (virSecurityDACSetOwnership(dev->data.file.path, priv->user, priv->group) < 0) + goto done; + } else { + if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || + (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { + virReportOOMError(); + goto done; + } + if ((virSecurityDACSetOwnership(in, priv->user, priv->group) < 0) || + (virSecurityDACSetOwnership(out, priv->user, priv->group) < 0)) + goto done; } - if ((virSecurityDACSetOwnership(in, priv->user, priv->group) < 0) || - (virSecurityDACSetOwnership(out, priv->user, priv->group) < 0)) - goto done; ret = 0; break; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 216208e..476bbbf 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -733,14 +733,19 @@ SELinuxSetSecurityChardevLabel(virDomainObjPtr vm, break; case VIR_DOMAIN_CHR_TYPE_PIPE: - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { - virReportOOMError(); - goto done; + if (virFileExists(dev->data.file.path)) { + if (SELinuxSetFilecon(dev->data.file.path, secdef->imagelabel) < 0) + goto done; + } else { + if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || + (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { + virReportOOMError(); + goto done; + } + if ((SELinuxSetFilecon(in, secdef->imagelabel) < 0) || + (SELinuxSetFilecon(out, secdef->imagelabel) < 0)) + goto done; } - if ((SELinuxSetFilecon(in, secdef->imagelabel) < 0) || - (SELinuxSetFilecon(out, secdef->imagelabel) < 0)) - goto done; ret = 0; break; -- 1.7.4.4

2 1

[libvirt] [PATCH] tests: Update valgrind suppressions file
by Matthias Bolte 03 May '11

03 May '11

--- tests/.valgrind.supp | 58 +++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 51 insertions(+), 7 deletions(-) diff --git a/tests/.valgrind.supp b/tests/.valgrind.supp index 4af10b1..68cfa0c 100644 --- a/tests/.valgrind.supp +++ b/tests/.valgrind.supp @@ -258,14 +258,58 @@ Memcheck:Param capget(data) fun:capget + fun:* fun:capng_clear + fun:virClearCapabilities fun:__virExec fun:virExecWithHook - fun:virExec - fun:qemudProbeMachineTypes - fun:qemudCapsInitGuest - fun:qemudCapsInit - fun:qemudStartup - fun:virStateInitialize - fun:main +} +{ + libnlMemoryLeak1 + Memcheck:Leak + fun:malloc + fun:strdup + obj:/usr/lib/libnl.so.1.1 +} +{ + libnlMemoryLeak2 + Memcheck:Leak + fun:calloc + obj:/usr/lib/libnl.so.1.1 +} +{ + libselinuxMemoryLeak1 + Memcheck:Leak + fun:malloc + fun:getdelim + obj:/lib/libselinux.so.1 +} +{ + dashMemoryLeak1 + Memcheck:Leak + fun:malloc + obj:/bin/dash +} +{ + dashMemoryLeak2 + Memcheck:Leak + fun:malloc + fun:strdup + obj:/bin/dash +} +{ + vboxMemoryLeak1 + Memcheck:Leak + ... + fun:VBoxNsxpNS_InitXPCOM2 +} +{ + libnetcfMemoryLeak1 + fun:malloc + fun:xmlStrndup + fun:xmlHashUpdateEntry3 + fun:* + fun:xsltRegisterAllExtras + fun:drv_init + fun:interfaceOpenInterface } -- 1.7.0.4

3 5

[libvirt] [PATCH] spice: support streaming-video parameter
by Alon Levy 03 May '11

03 May '11

This adds a streaming-video=filter|all|off attribute. It is used to change the behavior of video stream detection in spice, the default is filter (the default for libvirt is not to specify it - the actual default is defined in libspice-server.so). Usage: <graphics type='spice' autoport='yes'> <streaming mode='off'/> </graphics> Tested with the above and with tests/qemuxml2argvtest. Signed-off-by: Alon Levy <alevy(a)redhat.com> bla --- docs/schemas/domain.rng | 12 ++++++++ src/conf/domain_conf.c | 30 ++++++++++++++++++++ src/conf/domain_conf.h | 11 +++++++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 3 ++ .../qemuxml2argv-graphics-spice.args | 2 +- .../qemuxml2argv-graphics-spice.xml | 1 + 7 files changed, 60 insertions(+), 1 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 7163c6e..9083ff9 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1334,6 +1334,18 @@ <empty/> </element> </optional> + <optional> + <element name="streaming"> + <attribute name="mode"> + <choice> + <value>filter</value> + <value>all</value> + <value>off</value> + </choice> + </attribute> + <empty/> + </element> + </optional> </interleave> </group> <group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a681d9..eb9f587 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -352,6 +352,13 @@ VIR_ENUM_IMPL(virDomainGraphicsSpicePlaybackCompression, "on", "off"); +VIR_ENUM_IMPL(virDomainGraphicsSpiceStreamingMode, + VIR_DOMAIN_GRAPHICS_SPICE_STREAMING_MODE_LAST, + "default", + "filter", + "all", + "off"); + VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST, "subsystem", "capabilities") @@ -4082,6 +4089,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int flags) { VIR_FREE(compression); def->data.spice.playback = compressionVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "streaming")) { + const char *mode = virXMLPropString(cur, "mode"); + int modeVal; + + if (!mode) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("spice streaming missing mode")); + goto error; + } + if ((modeVal = + virDomainGraphicsSpiceStreamingModeTypeFromString(mode)) <= 0) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown spice streaming mode")); + VIR_FREE(mode); + goto error; + + } + VIR_FREE(mode); + + def->data.spice.streaming = modeVal; } } cur = cur->next; @@ -7979,6 +8006,9 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (def->data.spice.playback) virBufferVSprintf(buf, " <playback compression='%s'/>\n", virDomainGraphicsSpicePlaybackCompressionTypeToString(def->data.spice.playback)); + if (def->data.spice.streaming) + virBufferVSprintf(buf, " <streaming mode='%s'/>\n", + virDomainGraphicsSpiceStreamingModeTypeToString(def->data.spice.streaming)); } if (children) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1dadf98..7a1f29a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -696,6 +696,15 @@ enum virDomainGraphicsSpicePlaybackCompression { VIR_DOMAIN_GRAPHICS_SPICE_PLAYBACK_COMPRESSION_LAST }; +enum virDomainGraphicsSpiceStreamingMode { + VIR_DOMAIN_GRAPHICS_SPICE_STREAMING_MODE_DEFAULT = 0, + VIR_DOMAIN_GRAPHICS_SPICE_STREAMING_MODE_FILTER, + VIR_DOMAIN_GRAPHICS_SPICE_STREAMING_MODE_ALL, + VIR_DOMAIN_GRAPHICS_SPICE_STREAMING_MODE_OFF, + + VIR_DOMAIN_GRAPHICS_SPICE_STREAMING_MODE_LAST +}; + typedef struct _virDomainGraphicsDef virDomainGraphicsDef; typedef virDomainGraphicsDef *virDomainGraphicsDefPtr; struct _virDomainGraphicsDef { @@ -737,6 +746,7 @@ struct _virDomainGraphicsDef { int jpeg; int zlib; int playback; + int streaming; } spice; } data; }; @@ -1476,6 +1486,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceImageCompression) VIR_ENUM_DECL(virDomainGraphicsSpiceJpegCompression) VIR_ENUM_DECL(virDomainGraphicsSpiceZlibCompression) VIR_ENUM_DECL(virDomainGraphicsSpicePlaybackCompression) +VIR_ENUM_DECL(virDomainGraphicsSpiceStreamingMode) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainSeclabel) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1b22be6..2e25202 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -273,6 +273,8 @@ virDomainGraphicsSpicePlaybackCompressionTypeFromString; virDomainGraphicsSpicePlaybackCompressionTypeToString; virDomainGraphicsSpiceZlibCompressionTypeFromString; virDomainGraphicsSpiceZlibCompressionTypeToString; +virDomainGraphicsSpiceStreamingModeTypeFromString; +virDomainGraphicsSpiceStreamingModeTypeToString; virDomainGraphicsTypeFromString; virDomainGraphicsTypeToString; virDomainHostdevDefFree; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2205ed1..8036f0c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4037,6 +4037,9 @@ qemuBuildCommandLine(virConnectPtr conn, if (def->graphics[0]->data.spice.playback) virBufferVSprintf(&opt, ",playback-compression=%s", virDomainGraphicsSpicePlaybackCompressionTypeToString(def->graphics[0]->data.spice.playback)); + if (def->graphics[0]->data.spice.streaming) + virBufferVSprintf(&opt, ",streaming-video=%s", + virDomainGraphicsSpiceStreamingModeTypeToString(def->graphics[0]->data.spice.streaming)); virCommandAddArg(cmd, "-spice"); virCommandAddArgBuffer(cmd, &opt); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args index 70cd35b..084a100 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args @@ -4,6 +4,6 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ x509-dir=/etc/pki/libvirt-spice,tls-channel=main,plaintext-channel=inputs,\ image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\ -playback-compression=on -vga \ +playback-compression=on,streaming-video=filter -vga \ qxl -global qxl.vram_size=18874368 -device qxl,id=video1,vram_size=33554432,bus=pci.0,addr=0x4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml index a29f50d..0d3dd48 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml @@ -28,6 +28,7 @@ <jpeg compression='auto'/> <zlib compression='auto'/> <playback compression='on'/> + <streaming mode='filter'/> </graphics> <video> <model type='qxl' vram='18432' heads='1'/> -- 1.7.5

2 1

[libvirt] [PATCH v2] Fix disability to run on systems with no PCI bus
by Michal Privoznik 03 May '11

03 May '11

The patch which moved libpciaccess initialization to one place caused regression - we were not able to run on system with no PCI bus, like s390(x). --- src/node_device/node_device_udev.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 2139ef3..fcff252 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1421,8 +1421,12 @@ static int udevDeviceMonitorShutdown(void) ret = -1; } +#if defined __s390__ || defined __s390x_ + /* Nothing was initialized, nothing needs to be cleaned up */ +#else /* pci_system_cleanup returns void */ pci_system_cleanup(); +#endif return ret; } @@ -1595,6 +1599,10 @@ static int udevDeviceMonitorStartup(int privileged) int ret = 0; int pciret; +#if defined __s390__ || defined __s390x_ + /* On x390(x) system there is no PCI bus. + * Therefore there is nothing to initialize here. */ +#else if ((pciret = pci_system_init()) != 0) { /* Ignore failure as non-root; udev is not as helpful in that * situation, but a non-privileged user won't benefit much @@ -1607,6 +1615,7 @@ static int udevDeviceMonitorStartup(int privileged) goto out; } } +#endif if (VIR_ALLOC(priv) < 0) { virReportOOMError(); -- 1.7.4.4

3 2

[libvirt] [PATCH] fix missing VLAN id for Qbg example
by Gerhard Stenzel 03 May '11

03 May '11

For IEEE 802.1Qbg, it is necessary to use a VLAN interface. vepa itself does not require a VLAN interface. Signed-off-by: Gerhard Stenzel <stenzel at de.ibm.com> =================================================================== --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1440,6 +1440,10 @@ the Virtual Station Interface (VSI) represents the virtual interface of a virtual machine. </p> + <p> + Please note, that IEEE 802.1Qbg requires a non-zero value for the + VLAN ID. + </p> <dl> <dt><code>managerid</code></dt> <dd>The VSI Manager ID identifies the database containing the VSI type @@ -1466,7 +1470,7 @@ <interface type='direct'/> ... <interface type='direct'> - <source dev='eth0' mode='vepa'/> + <source dev='eth0.2' mode='vepa'/> <virtualport type="802.1Qbg"> <parameters managerid="11" typeid="1193047" typeidversion="2" instanceid="09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f"/> </virtualport> =================================================================== Best regards, Gerhard Stenzel ------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

4 5

[libvirt] [PATCH] tests: avoid compiler warning
by Eric Blake 02 May '11

02 May '11

../../tests/xmconfigtest.c: In function 'testCompareParseXML': ../../tests/xmconfigtest.c:49:19: error: 'conn' may be used uninitialized in this function [-Wuninitialized] * tests/xmconfigtest.c (testCompareParseXML): Initialize variable. --- Pushing under the build-breaker rule. tests/xmconfigtest.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index 02a8900..6022621 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -46,7 +46,7 @@ testCompareParseXML(const char *xmcfg, const char *xml, int xendConfigVersion) char *gotxmcfgData = NULL; virConfPtr conf = NULL; int ret = -1; - virConnectPtr conn; + virConnectPtr conn = NULL; int wrote = 4096; struct _xenUnifiedPrivate priv; virDomainDefPtr def = NULL; -- 1.7.4.4

1 0

[libvirt] Libvirt 0.9.1 week freeze, RC1 version to test
by Daniel Veillard 02 May '11

02 May '11

So we are entering the week freeze for the new version. I have made an rc1 tarball available at: ftp://libvirt.org/libvirt/libvirt-0.9.1-rc1.tar.gz I have tried it locally and basic operations seems to work fine, please give it a try too, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

4 7

[libvirt] [PATCH] qemu: fix uninitialized variable warning
by Christophe Fergeau 02 May '11

02 May '11

This commit fixes qemu/qemu_driver.c: In function 'qemuDomainModifyDeviceFlags': qemu/qemu_driver.c:4041:8: warning: 'ret' may be used uninitialized in this function [-Wuninitialized] qemu/qemu_driver.c:4013:9: note: 'ret' was declared here The variable is set to -1 so that the error paths are taken when the code to set it didn't get a chance to run. Without initializing it, we could return some an undefined value from this function. While I was at it, I made a trivial whitespace change in the same function to improve readability. --- src/qemu/qemu_driver.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a19c4a7..16d869d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4010,11 +4010,11 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, { virDomainDiskDefPtr disk = dev->data.disk; virCgroupPtr cgroup = NULL; - int ret; + int ret = -1; if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, - vm->def->name, &cgroup, 0) !=0 ) { + vm->def->name, &cgroup, 0) != 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to find cgroup for %s"), vm->def->name); -- 1.7.5

2 1

[libvirt] Libvirt and IPSec (was: What about Trusted Virtual Domains???)
by Paolo Smiraglia 02 May '11

02 May '11

Hi to everyone! First of all, sorry for the thread subject change. Due to the several issues of the Libvirt implementation of the Trusted Virtual Domains (TVD), I decided to approach the topic in a modular manner. I think that the first step should be to define the IPSec support or, more in general, the secure tunnel support for Libvirt. I see the implementation divided in two step: 1. define a new driver called 'sectunnel' which describes a generic secure tunnel that will be established using several technologies (for now using only ipsec) 2. modify the existing 'network' driver by adding the possibility to specify the 'sectunnel' that the network have to use in the virtual network definition As an example, you can see below a possible XML definition of the network which use a secure tunnel and the corresponding 'sectunnel' XML definition: NETWORK DEFINITION ================== <network> <name>sec-net</name> <uuid>3e3fce45-4f53-4fa7-bb32-11f34168b82b</uuid> <bridge name='virbr0' /> <domain name='example' /> ... <sectunnel name='sec-tun' /> <--(specify the 'sectunnel' to use) </network> SECTUNNEL DEFINITION ==================== <sectunnel type='ipsec'> <name>sec-tun</name> <uuid>8b7fd1b0-4463-43b7-8b6e-8006344aeb66</uuid>  <sa> <secret uuid='...' /> <--(specify the 'secret' which contains the pre-shared key) </sa>  <sp> <src_range address='10.0.0.1' prefixlen='30' port='5000' /> <dst_range address='10.0.0.2' prefixlen='30' port='5000' /> <upperspec protocol='any' /> <policy direction='out' action='ipsec'> <rule protocol='esp' mode='tunnel' level='require'> <src address='192.168.0.1' port='55055' /> <dst address='192.168.0.2' port='55055' /> </rule> </policy> </sp> <sp> <src_range address='10.0.0.2' prefixlen='30' port='5000' /> <dst_range address='10.0.0.1' prefixlen='30' port='5000' /> <upperspec protocol='any' /> <policy direction='in' action='ipsec'> <rule protocol='esp' mode='tunnel' level='require'> <src address='192.168.0.2' port='55055' /> <dst address='192.168.0.1' port='55055' /> </rule> </policy> </sp> </sectunnel> As you can see in the 'sectunnel' XML definition, I use a 'secret' element. This element is a Libvirt secret [1] and it stores the pre-shared key used by IPSec to establish the Security Associations (SA). Obviously this feature requires to define a new usage category in the 'secret' driver definition. Another possible way to establish the SA is to use the X.509 certificates. To this purpose, I think that the certificates already used by Libvirt to setup SSL/TLS remote connections, might be used. That's all! :-) What do you think about this possible IPSec implementation? Thanks in advance for the replies! Best regards, PAOLO LINK LIST --------- [1] http://libvirt.org/formatsecret.html -- PAOLO SMIRAGLIA Department of Control and Computer Engineering Mobile: +39 (333) 527 3593 Email: paolo.smiraglia(a)polito.it

3 12

[libvirt] RFC: virInterface change transaction API
by Laine Stump 02 May '11

02 May '11

I've been asked to implement what some people have termed as a "transaction-oriented" API for host interface configuration (ie virInterface*()). The basic intent is to allow rollback to a known-good config if anything goes wrong when changing around the host network config with virInterface*() functions. The most straightforward way to achieve this is that prior to calling virInterfaceDefine/virInterfaceUndefine, the current state of the host's network configuration (ie the /etc/sysconfig/network-scripts/ifcfg-* files in the case of Fedora and RHEL) would be saved off somewhere, and kept around until we're sure the new config is good; once we know that, we can just eliminate the backup. If, however, the user of virInterface*() explicitly requests, we could copy the files back; alternately if the system is rebooted without these known-good files being erased, we would assume that something went wrong and restore the original config. As with all other virInterface functions, the details of all this will be handled by netcf (and below), but since libvirt is the main consumer of netcf, I figure this is the appropriate place to discuss how it gets done, so please let me know any opinions on any piece of this. I plan to start the implementation "soon", as I want to be finished before the end of May. I see 3 layers to this: 1) libvirt At the libvirt layer, this feature just requires 3 new APIs, which are directly passed through to netcf: virInterfaceChangeStart(virConnectPtr conn, unsigned int flags); virInterfaceChangeCommit(virConnectPtr conn, unsigned int flags); virInterfaceChangeRollback(virConnectPtr conn, unsigned int flags); For the initial implementation, these will be simple passthroughs to similarly named netcf functions. (in the future, it would be useful for the server side of libvirt to determine if client<->server connectivity was lost due to the network changes, and automatically tell netcf to do a rollback). 2) netcf The netcf api will have these same three APIs, just named slightly differently: ncf_change_start(struct netcf *ncf, unsigned int flags); There are two possibilities for this. Either: A) call the initscript described below to save all config files that might possibly be changed (snapshot_config) or B) set a flag in *ncf indicating that all future calls to netcf that would end up modifying a particular config file should save off that file *if it hasn't already been saved*. (A) is simpler, but relies on the initscript having exact/complete matching knowledge of what files netcf may change. Should we worry about that and deal with the complexities of (B), or is (A) good enough for now? ncf_change_rollback(struct netcf *ncf, unsigned int flags); Again, two possbilities: A) a) save the config of all current interfaces (in memory) b) call the initscript below to restore the config to its original state. c) compare the new config to the old, and: * bring down any interfaces that no longer exist (PROBLEM: once an interface has no config files, you can no longer operate on it with "ifdown") * bounce any interfaces that have changed * bring up any interfaces that have been re-added or B) a) ifdown all interfaces b) call initscript to restore previous config (rollback_config) c) ifup all interfaces. (A) is much simpler, but may lead to unnecessary difficulties when we bounce interfaces that didn't really need it. So, the same question oas for ncf_change_start() - is the more exact operation worth the extra complexity? ncf_change_commit(struct netcf *ncf, unsigned int flags); The simplest function - this will just call the initscript to erase the backup (commit_config). 3) initscript This initscript will at first live in (be installed by) netcf (called /etc/init.d/networking-config?), but hopefully it will eventually be accepted by the initscripts package (which includes the networking-related initscripts), as it is of general use. (Dan Kenigsberg already already took a stab at this script last year, but received no reply from the initscripts maintainers, implying they may not be too keen on the idea right now - it might take some convincing ;-) https://fedorahosted.org/pipermail/initscripts-devel/2010-February/000025.h… It will have three commands, one of which will be called automatically by "start" (the command called automatically at boot time): snapshot_config This will save a copy of (what the script believes are - is this problematic?) all network-config related files. It may or may not be called by netcf (see the notes in ncf_start_change() above. If this function finds that a snapshot has already been taken, it should fail. rollback_config (automatically called from "start" at boottime) This will move back (from the saved copies) all files that were changed/removed since snapshot, *and delete any files that have been added*. Note that this command doesn't need to worry about ifup/ifdown, because it will be called prior to any other networking startup (part of the reason that netcf will need to deal with that). I notice that Dan K's version saves the modified files to a "rollback-${date}" directory. Does this seem like a good idea? It's nice to not lose anything, but there is no provision for eliminating old versions, so it could grow without bound. commit_config This will just remove all the files in the save directory. So, the two problems I have right now: 1) Do we accept the inexact method of just saving all files that match a list of patterns during *start(), then in *rollback() erasing all files matching that pattern and copying the old file back? Or do we need to keep track of what files have been changed/removed and added, and copy back / delete only those files during rollback? (A version control system would keep track of this rather nicely, but that's too complex for something that's intended to be a failsafe (and that we would also like to eventually be in the base OS install). Dan B. at one point suggested using patchfiles if I wanted the save info to keep exact track of which files would need to be replaced/deleted on rollback, but on further thought this turns out to not be workable, since we would need to run diff (to create the patchfile) after all changes had been made, and any outside changes to any of the files would leave the patchfile un-appliable, thus causing our "failsafe" to fail :-( ). Therefore, we will need to rely on the list of globs to tell us what files need to be deleted, or keep our own list in a separate file.) 2) Is it going to be okay to ifdown all interfaces prior to the rollback, and ifup all interfaces afterwards? Or must we compare the new config to the original, and ifdown only those interfaces that had been previously added/changed, then ifup only those interfaces that had been previously removed/changed? 3) If anyone has ideas on making the initscript more palatable to the initscripts people, please speak up! :-) (one comment from an initscripts person was that 1) for the general case it would be difficult to draw the line on what parts of network connectivity should be included in this rollback functionality, and 2) at some point this becomes a general system config problem, and would really be better addressed by a general system wide config management system. These are both concerns that need well qualified answers. (I tend to think that this is intended as a failsafe to prevent unreachable systems, so it should be as simple as possible, and thus shouldn't be burdened with the complexity of a full system config management system (which could also co-exist at a higher level), but better answers are welcome.)

4 10