<div dir="ltr">I built a test domain using the default storage pool, backed by files under /var/lib/libvirt/images/. I can confirm that the apparent race condition also appears there, too. So it definitely seems like using volumes in general can cause issues with domains being restarted after a libvirt restart. I can post more of the debug-enabled log file if anyone wants, but the error message is "qemuTranslateDiskSourcePool:1376 : unsupported configuration: storage pool 'default' containing volume 'test.img' is not active."<div>
<br></div><div>In my troubleshooting, the race condition seemed to reboot at least one domain (out of three) about 40% of the time. My troubleshooting also seems to confirm that my suspicion that this bug had been more or less hidden by the fact that libvirt previously refused to take over an existing domain with RBD pools, combined with the seemingly low adoption rate of storage pools in frontend software thus far. I'll try and think about and implement possible solutions to this race condition over the weekend.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Dec 6, 2013 at 4:40 PM, Adam Walters <span dir="ltr"><<a href="mailto:adam@pandorasboxen.com" target="_blank">adam@pandorasboxen.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Unfortunately, after I got home, I discovered that this patch introduces a bug that seems to have been previously hidden behind the one that this fixes. Doubly unfortunate, is that domains using RBD (and probably ISCSI, but I don't have the facilities to test this) volumes with authentication can't be reconnected upon libvirt restart without this patch (or something similarly bypassing or fixing the check on actual vs expected secret usage). If all four of my proposed patches are accepted, an errata entry would likely be needed to note that using network volumes with authentication may (or really will almost certainly) cause those domains to be rebooted. Below are my findings and thoughts on the problem I'm seeing. Would anyone else have suggestions on how to resolve this?</div>
<div><br></div><div>During libvirt startup, there seems to be a race condition while the various state driver subsystems are initialized and auto-start is run. It seems to be that all state drivers are initialized, then once they are all initialized, the auto-start function is run (if defined) for each of them. Due to this behavior, most of the time I restart libvirt while using storage pool volumes (currently I only have RBD volumes in use. I need to add a domain using a file-backed volume to test that as well), any running domains that had been using a volume are restarted due to the storage pool not being active when it tries to reconnect to the running domain. The odd part is that occasionally, my domains don't restart. I have yet to get a debug log from this, but I suspect it is related to the order in which the state drivers are loaded and auto-started. Either that or occasionally QEMU takes longer to get around to reconnecting to the monitor socket.<br>
</div><div><br></div><div>As far as a resolution for the apparent race condition, I am a bit low on suggestions at the moment. I am assuming that there is a reason that state drivers are all loaded, then all auto-started. If not, it could be as simple as running auto-start for a state driver immediately after loading it. Another possible solution might be to enforce an ordering on state drivers. Perhaps forcing virtualization state drivers (such as QEMU, LXC, Xen, etc) to be initialized and auto-started after all other state drivers have completed their sequences. I'm not sure where or how this list is created, though. Since this occasionally works without restarting domains, I do assume that the list is either generated at run-time and/or the initialization and auto-start sequences are asynchronous. Unfortunately, my experience with asynchronous code is about nil, so I'm not sure how much help I could provide in removing a race condition.</div>
<div><br></div><div>An alternate solution that, while not pretty, may be better than a reboot of the domains, would be to change the behavior when QEMU attempts a reconnect and finds the storage pool inactive. Instead of terminating the domain, perhaps it could be paused. The auto-start would then resume it if the pool was active at that point, otherwise, it would remain paused until someone resumed it.</div>
<div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Dec 6, 2013 at 3:10 PM, Adam Walters <span dir="ltr"><<a href="mailto:adam@pandorasboxen.com" target="_blank">adam@pandorasboxen.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">While testing Peter's changes to my patches, I discovered that when<br>
restarting libvirt with domains running, they would no longer show<br>
up when a 'virsh list' was issued. I tracked this bug down to this<br>
section of code within virDomainDiskDefParseXML. The symptom was<br>
that libvirt was issuing an internal error saying that the secret<br>
type was invalid. This is caused because this function didn't handle<br>
storage pool volumes. I do not have any ISCSI volumes with which to<br>
test, but it would theoretically affect those, as well. I attempted<br>
to make the function smarter, by checking the pool type and setting<br>
expected_secret_usage properly, but this function does not have a<br>
valid connection pointer, and I was thus unable to do so.<br>
---<br>
src/conf/domain_conf.c | 3 ++-<br>
1 file changed, 2 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c<br>
index 332cb50..82587eb 100644<br>
--- a/src/conf/domain_conf.c<br>
+++ b/src/conf/domain_conf.c<br>
@@ -5311,7 +5311,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,<br>
cur = cur->next;<br>
}<br>
<br>
- if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage) {<br>
+ if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage &&<br>
+ def->type != VIR_DOMAIN_DISK_TYPE_VOLUME) {<br>
virReportError(VIR_ERR_INTERNAL_ERROR,<br>
_("invalid secret type '%s'"),<br>
virSecretUsageTypeTypeToString(auth_secret_usage));<br>
<span><font color="#888888">--<br>
1.8.4.2<br>
<br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div><br></div>