| schopin | slyon: I stumbled across something strange when working on the parsing code : https://github.com/canonical/netplan/blob/main/src/parse.c#L1672 | 08:03 |
|---|---|---|
| schopin | can you confirm that if the API client passes error = NULL, which is totally valid, the parser will silently swallow up errors as long as it has successfully parsed to and from ? | 08:04 |
| schopin | i.e. [ { from: "192.168.1.1", to: "10.3.47.75", fwmark: bogus }, { ... a valid rule object } ] should return TRUE if we don't have a GError at hand. | 08:07 |
| slyon | schopin: indeed, I can confirm this is the current behaviour | 08:12 |
| slyon | well... | 08:15 |
| slyon | but the other values are checked in their handler, no? like mark/fwmark would be checked in handle_generic_guint and return yaml_error() if it is bogus | 08:16 |
| schopin | sure, but said value isn't stored anywhere, so if there are no GError provided we just don't append the faulty rule and go on to the next, as (error && *error) will always be false. | 08:19 |
| schopin | I'm starting to think that `if (error && *error) return FALSE;` is an antipattern. It's like a CLI tool that would change its return code if we pass in `-q` as an option :P | 08:22 |
| slyon | right. if process_mapping fails the cur_ip_rule is not appended to the cur_netdef->ip_rules array and leaks memory instead.. That should probably be fixed. | 08:29 |
| schopin | There are leaks all over on error cases, they should be fixed in #233 | 08:31 |
| schopin | Basically my question boils down to "is the client supposed to give us a valid GError pointer at the entry point" ? | 08:31 |
| schopin | Note that netplan_finish_parse has the same issue. If called like this `netplan_finish_parse(NULL)`, any error raised by finish_iterator will be ignored : https://github.com/canonical/netplan/blob/main/src/parse.c#L2676 | 08:39 |
| slyon | Yeah. I think that is kind of the expected behaviour (sans the memory leaks). If the client passes error=NULL he wants to ignore any errors. | 08:41 |
| schopin | I find this a bit weird. The API supposedly already gives us room to signal the caller that something went wrong (return FALSE or NULL), and I understand the GError as simply a way to get more info. | 08:45 |
| schopin | Plus, this behaviour isn't consistent. You cannot ignore an error when parsing an ARP IP target (just below), for instance. | 08:47 |
| slyon | Agreed. You're right. | 08:48 |
| slyon | the API should provide proper return values if parsing failed. and GError should be used to provide the extra info. | 08:50 |
| schopin | ACK, I'll post a PR for this soon. Do you mind if I ignore the leaks in it though? | 08:51 |
| slyon | Thank you schopin! You can ignore the leaks for now, but maybe mark them with an "XXX: fix memory leak" comment while on it | 09:07 |
| Daulity | hey | 10:09 |
| Daulity | trying to configure a static address to an interface with netplan but after a apply the interface is assigned nothing | 10:10 |
| slyon | Daulity: can you paste your config somewhere (e.g. paste.ubuntu.com)? and run `netplan --debug apply` to see if it shows any errors? | 10:10 |
| Daulity | this is the config: https://termbin.com/2txcd | 10:12 |
| Daulity | all that should happen on that interface is set an ip address and a netmask | 10:16 |
| Daulity | this is the debug output: https://termbin.com/qehi | 10:20 |
| slyon | Daulity: yes that config looks correct. Could you please also show the output of `ip a`? | 10:47 |
| slyon | does the config apply after a reboot? | 10:48 |
| Daulity | slyon: i haven't tried a reboot yet will do | 11:36 |
| Daulity | here is ip a output: https://termbin.com/kwnd | 11:37 |
| Daulity | doesn't apply after a reboot | 11:40 |
| slyon | Daulity: intersting... could you please check/share the output of `networkctl status eth0`? | 11:53 |
| slyon | its all looking correct so far. I wonder if there are some manual systemd-networkd configuration in place or systemd-networkd does not work correctly for another reason? | 11:54 |
| Daulity | slyon: it's not plugged in on that port but that never mattered in the past | 12:12 |
| Daulity | https://termbin.com/fvf4d3 | 12:15 |
| slyon | Daulity: aha! that explains the situation. Systemd won't configure the interface if it has no carrier. We've recently landed a change upstream to allow specifying `ignore-carrier=true` in your netplan config: https://github.com/canonical/netplan/commit/d4884cfd40e1e33540b274371c3272df6595d22c | 12:20 |
| slyon | That change will be included in the upcoming netplan release. For now you should be able to create a systemd-networkd drop-in config, e.g. in /etc/systemd/network/netplan-eth0.network.d/override.conf that includes: | 12:21 |
| slyon | [Network] | 12:21 |
| slyon | ConfigureWithoutCarrier=yes | 12:21 |
| Daulity | I don't actually know if it is a problem | 12:39 |
| Daulity | because if i plug in a network cable it's configured correctly and I think that is all that matters :) | 12:39 |
| slyon | ack | 12:44 |
| === Beret- is now known as Beret | ||
| === dbungert1 is now known as dbungert | ||
| === JanC_ is now known as JanC | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!