Use slave-type instead of port-type for NM<1.46.0 #2433
Conversation
| } else { | ||
| connection_dbus.insert("port-type", "".into()); | ||
| if VersionReq::parse(">=1.46.0").unwrap().matches(&nm_version) { | ||
| connection_dbus.insert("port-type", "".into()); | ||
| } else { | ||
| connection_dbus.insert("slave-type", "".into()); | ||
| } | ||
| connection_dbus.insert("master", "".into()); | ||
| } |
There was a problem hiding this comment.
Btw, I might be missing something but isn't this nonsensical to have, because it seems to me to get cleaned up later by cleanup_dbus_connection() anyway? So should I maybe just remove this part completely?
There was a problem hiding this comment.
Mhm maybe it's relevant for merging connections? Though if the empty value then gets cleaned up before it's sent to NM, NM will just keep the value as is anyway so it will also not be unset/updated.
There was a problem hiding this comment.
Yes, I have the same impression. That value will be cleaned-up later. I guess we need to have a closer look to this.
There was a problem hiding this comment.
I found some issue when working on this exactly because of the merge and also when for example moving a port from a bond to a bridge. It was reading the port-type but setting the slave-type or something like that..
There was a problem hiding this comment.
Okay so I tested it a bit, indeed the merging seems to be a problem because for whatever reason NM still seems to send slave-type over port-type to agama and then the merging gets difficult, so I have now added a bit in cleanup_dbus_connection() that prefers the port-type of agama so that it takes precedence. And also deleting the section I mentioned here does not work because then the merge wouldn't overwrite the values/cleanup_dbus_connection() wouldn't see a change. But also what I mentioned previously
Though if the empty value then gets cleaned up before it's sent to NM, NM will just keep the value as is anyway so it will also not be unset/updated.
isn't correct in this case, if nothing is sent to NM it will actually unset this property, so cleanup_dbus_connection() is working as intended.
There was a problem hiding this comment.
Thanks for checking it. Then I guess we can proceed.
jcronenberg
force-pushed
the
slave_type_nm_compat
branch
from
81fdb9d to
39b35b1
Compare
jcronenberg
force-pushed
the
slave_type_nm_compat
branch
from
39b35b1 to
77b12ad
Compare
Prepare to release Agama 16: * #1868 * #2347 * #2356 * #2373 * #2393 * #2402 * #2404 * #2406 * #2408 * #2409 * #2410 * #2411 * #2412 * #2413 * #2414 * #2415 * #2416 * #2417 * #2418 * #2419 * #2420 * #2421 * #2422 * #2423 * #2424 * #2425 * #2426 * #2427 * #2428 * #2431 * #2433 * #2434 * #2436 * #2437 * #2438 * #2439 * #2440 * #2441 * #2442 * #2443 * #2445 * #2446 * #2450 * #2451 * #2452 * #2453 * #2454 * #2455 * #2456 * #2457 * #2458 * #2460 * #2461 * #2462 * #2463 * #2464 * #2465 * #2466 * #2467 * #2468 * #2469 * #2470 * #2471 * #2472 * #2473 * #2474 * #2475 * #2476 * #2478 * #2479 * #2480 * #2482 * #2483 * #2484 * #2485 * #2487 * #2488 * #2489 * #2490 * #2491 * #2493 * #2494 * #2495 * #2496 * #2497 * #2498 * #2499 * #2502 * #2505 * #2507 * #2509 * #2511 * #2512 * #2513 * #2515 * #2516 * #2517 * #2518 * #2520 * #2523 * #2524 * #2525
3 participants
Problem
With #2356 I unfortunately missed the changes from #2258 where
slave-typewas changed toport-typewhich only NM >= 1.46.0 supports.Solution
Send
port-typeonly for NM >= 1.46.0, otherwiseslave-typeis used.Also since I was already adding a test for this, I also added two additional test for
connection_to_dbuswith bridge and bond connections.Testing