Fix issue 2513 #3148
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ package kvm | |
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "encoding/xml" | ||
| "fmt" | ||
| "io/ioutil" | ||
| "strings" | ||
|
|
@@ -43,11 +44,15 @@ const networkTmpl = ` | |
| </network> | ||
| ` | ||
|
|
||
| // setupNetwork ensures that the network with `name` is started (active) | ||
| // and has the autostart feature set. | ||
| func setupNetwork(conn *libvirt.Connect, name string) error { | ||
| n, err := conn.LookupNetworkByName(name) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "checking network %s", name) | ||
| } | ||
|
|
||
| // always ensure autostart is set on the network | ||
| autostart, err := n.GetAutostart() | ||
| if err != nil { | ||
| return errors.Wrapf(err, "checking network %s autostart", name) | ||
|
|
@@ -58,6 +63,7 @@ func setupNetwork(conn *libvirt.Connect, name string) error { | |
| } | ||
| } | ||
|
|
||
| // always ensure the network is started (active) | ||
| active, err := n.IsActive() | ||
| if err != nil { | ||
| return errors.Wrapf(err, "checking network status for %s", name) | ||
|
|
@@ -67,52 +73,175 @@ func setupNetwork(conn *libvirt.Connect, name string) error { | |
| return errors.Wrapf(err, "starting network %s", name) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // ensureNetwork is called on start of the VM | ||
| func (d *Driver) ensureNetwork() error { | ||
| conn, err := getConnection() | ||
| if err != nil { | ||
| return errors.Wrap(err, "getting libvirt connection") | ||
| } | ||
| defer conn.Close() | ||
|
|
||
| // network: default | ||
|
|
||
| // Start the default network | ||
| // It is assumed that the libvirt/kvm installation has already created this network | ||
| log.Infof("Ensuring network %s is active", defaultNetworkName) | ||
| if err := setupNetwork(conn, defaultNetworkName); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // network: private | ||
|
|
||
| // Start the private network | ||
| log.Infof("Ensuring network %s is active", d.PrivateNetwork) | ||
| if err := setupNetwork(conn, d.PrivateNetwork); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // createNetwork is called during creation of the VM only (and not on start) | ||
| func (d *Driver) createNetwork() error { | ||
| conn, err := getConnection() | ||
| if err != nil { | ||
| return errors.Wrap(err, "getting libvirt connection") | ||
| } | ||
| defer conn.Close() | ||
|
|
||
| // network: default | ||
| // It is assumed that the libvirt/kvm installation has already created this network | ||
|
|
||
| // network: private | ||
|
|
||
| // Only create the private network if it does not already exist | ||
| if _, err := conn.LookupNetworkByName(d.PrivateNetwork); err != nil { | ||
| // create the XML for the private network from our networkTmpl | ||
| tmpl := template.Must(template.New("network").Parse(networkTmpl)) | ||
| var networkXML bytes.Buffer | ||
| if err := tmpl.Execute(&networkXML, d); err != nil { | ||
| return errors.Wrap(err, "executing network template") | ||
| } | ||
|
|
||
| // define the network using our template | ||
| network, err := conn.NetworkDefineXML(networkXML.String()) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "defining network from xml: %s", networkXML.String()) | ||
| } | ||
|
|
||
| // and finally create it | ||
| if err := network.Create(); err != nil { | ||
| return errors.Wrapf(err, "creating network %s", d.PrivateNetwork) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (d *Driver) deleteNetwork() error { | ||
| type source struct { | ||
| //XMLName xml.Name `xml:"source"` | ||
| Network string `xml:"network,attr"` | ||
| } | ||
| type iface struct { | ||
| //XMLName xml.Name `xml:"interface"` | ||
| Source source `xml:"source"` | ||
| } | ||
| type result struct { | ||
| //XMLName xml.Name `xml:"domain"` | ||
| Name string `xml:"name"` | ||
| Interfaces []iface `xml:"devices>interface"` | ||
| } | ||
|
|
||
| conn, err := getConnection() | ||
| if err != nil { | ||
| return errors.Wrap(err, "getting libvirt connection") | ||
| } | ||
| defer conn.Close() | ||
|
|
||
| // network: default | ||
| // It is assumed that the OS manages this network | ||
|
|
||
| // network: private | ||
| log.Debugf("Checking if network %s exists...", d.PrivateNetwork) | ||
| network, err := conn.LookupNetworkByName(d.PrivateNetwork) | ||
| if err != nil { | ||
| // TODO: decide if we really wanna throw an error? | ||
| return errors.Wrap(err, "network %s does not exist") | ||
| } | ||
| log.Debugf("Network %s exists", d.PrivateNetwork) | ||
|
|
||
| // iterate over every (also turned off) domains, and check if it | ||
| // is using the private network. Do *not* delete the network if | ||
| // that is the case | ||
| log.Debug("Trying to list all domains...") | ||
| doms, err := conn.ListAllDomains(0) | ||
| if err != nil { | ||
| return errors.Wrap(err, "list all domains") | ||
| } | ||
| log.Debugf("Listed all domains: total of %d domains", len(doms)) | ||
|
|
||
| // fail if there are 0 domains | ||
| if len(doms) == 0 { | ||
| return fmt.Errorf("list of domains is 0 lenght") | ||
|
There was a problem hiding this comment. nit: typo. What's the correct fix here? It would be good to include some steps the user could take to create the domain. There was a problem hiding this comment. yeah ... good point to think about what this error means to the user ... it's one of those statements that will/should never happen ... I think there is two things we could do to address this:
I'd personally opt for (2), but I let you make that decision @dlorenc. What do you think? There was a problem hiding this comment. Let's not panic: https://github.com/golang/go/wiki/CodeReviewComments#dont-panic I like your #2 option, since this is just at the deletion stage, but just in case it helps a future debugger, do you mind putting a log.Warningf in about the unexpected case? Thanks again for your contribution. |
||
| } | ||
|
|
||
| for _, dom := range doms { | ||
| // get the name of the domain we iterate over | ||
| log.Debug("Trying to get name of domain...") | ||
| name, err := dom.GetName() | ||
| if err != nil { | ||
| return errors.Wrap(err, "failed to get name of a domain") | ||
| } | ||
| log.Debugf("Got domain name: %s", name) | ||
|
|
||
| // skip the domain if it is our own machine | ||
| if name == d.MachineName { | ||
| log.Debug("Skipping domain as it is us...") | ||
| continue | ||
| } | ||
|
|
||
| // unfortunately, there is no better way to retrieve a list of all defined interfaces | ||
| // in domains than getting it from the defined XML of all domains | ||
| // NOTE: conn.ListAllInterfaces does not help in this case | ||
| log.Debugf("Getting XML for domain %s...", name) | ||
| xmlString, err := dom.GetXMLDesc(libvirt.DOMAIN_XML_INACTIVE) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to get XML of domain '%s'", name) | ||
| } | ||
| log.Debugf("Got XML for domain %s", name) | ||
|
|
||
| v := result{} | ||
| err = xml.Unmarshal([]byte(xmlString), &v) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to unmarshal XML of domain '%s", name) | ||
| } | ||
| log.Debugf("Unmarshaled XML for domain %s: %#v", name, v) | ||
|
|
||
| // iterate over the found interfaces | ||
| for _, i := range v.Interfaces { | ||
| if i.Source.Network == d.PrivateNetwork { | ||
| log.Debugf("domain %s DOES use network %s, aborting...", name, d.PrivateNetwork) | ||
| return fmt.Errorf("network still in use at least by domain '%s',", name) | ||
| } | ||
| log.Debugf("domain %s does not use network %s", name, d.PrivateNetwork) | ||
| } | ||
| } | ||
|
|
||
| // when we reach this point, it means it is safe to delete the network | ||
| log.Debugf("Trying to destroy network %s...", d.PrivateNetwork) | ||
| err = network.Destroy() | ||
| if err != nil { | ||
| return errors.Wrap(err, "network destroy") | ||
| } | ||
| log.Debugf("Trying to undefine network %s...", d.PrivateNetwork) | ||
| err = network.Undefine() | ||
| if err != nil { | ||
| return errors.Wrap(err, "network undefine") | ||
| } | ||
|
|
||
| return nil | ||
|
|
@@ -136,6 +265,7 @@ func (d *Driver) lookupIP() (string, error) { | |
| return d.lookupIPFromLeasesFile() | ||
| } | ||
|
|
||
| // TODO: for everything > 1002006, there is direct support in the libvirt-go for handling this | ||
| return d.lookupIPFromStatusFile(conn) | ||
| } | ||
|
|
||
|
|
@@ -167,7 +297,7 @@ func (d *Driver) lookupIPFromStatusFile(conn *libvirt.Connect) (string, error) { | |
|
|
||
| ipAddress := "" | ||
| for _, status := range statusEntries { | ||
| if status.MacAddress == d.PrivateMAC { | ||
| ipAddress = status.IPAddress | ||
| } | ||
| } | ||
|
|
@@ -192,7 +322,7 @@ func (d *Driver) lookupIPFromLeasesFile() (string, error) { | |
| if len(entry) != 5 { | ||
| return "", fmt.Errorf("Malformed leases entry: %s", entry) | ||
| } | ||
| if entry[1] == d.PrivateMAC { | ||
| ipAddress = entry[2] | ||
| } | ||
| } | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: inline the if statement here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough, inlining this statement right now