arcnmx / qapi-rs

QEMU QMP and Guest Agent protocol for Rust
MIT License
44 stars 11 forks source link

"can only flatten structs and maps (got a string))" error from enum ObjectOptions #13

Closed gurugio closed 2 years ago

gurugio commented 2 years ago

Hi,

I tried to use object_add as below code snipet.

        self.vm.stream.execute(qmp::object_add {
            0: qmp::ObjectOptions::memory_backend_ram {
                id: "asdf".to_string(),
                memory_backend_ram: qmp::MemoryBackendProperties {
                    // compatible with only qapi-qmp v0.8.0
                    merge: None,
                    reserve: None,
                    size: 1024 * 1024 * 1024 * (gb as usize),
                    host_nodes: None,
                    x_use_canonical_path_for_ramblock_id: None,
                    dump: None,
                    prealloc: None,
                    share: None,
                    prealloc_threads: None,
                    policy: Some(qmp::HostMemPolicy::default),
                },
            },
        };

I got "can only flatten structs and maps (got a string))" error. So I checked the code of enum ObjectOptions in qapi-rs/target/debug/build/qapi-qmp-a7c0e267f5c0521a/out/qmp.rs file. And I found there is serde(flatten) attribute for id: String.

I guess serde cannot do flatten with String data. So I made a test code as below.

    #[test]
    fn check_flatten() {
        #[derive(Debug, Clone, Serialize, Deserialize)]
        struct MemoryBackendProperties {
            #[serde(rename = "size")]
            pub size: u64,
        }

        #[derive(Debug, Clone, Serialize, Deserialize)]
        #[serde(tag = "qom-type")]
        enum ObjectOptions {
            #[serde(rename = "memory-backend-ram")]
            memory_backend_ram {
                #[serde(flatten)]
                #[serde(rename = "id")]
                id: ::std::string::String,
                #[serde(flatten)]
                #[serde(rename = "memory-backend-ram")]
                memory_backend_ram: MemoryBackendProperties,
            },
        }

        let ddd = ObjectOptions::memory_backend_ram {
            id: "asdf".to_string(),
            memory_backend_ram: MemoryBackendProperties { size: 1234 },
        };
        println!("{:?}", ddd);

        let a = serde_json::to_string(&ddd).unwrap();
        println!("{}", a);
    }

My test generates the same error: "can only flatten structs and maps (got a string)".

running 1 test
memory_backend_ram { id: "asdf", memory_backend_ram: MemoryBackendProperties { size: 1234 } }
thread 'vm_controller::tests::check_flatten' panicked at 'called `Result::unwrap()` on an `Err` value: Error("can only flatten structs and maps (got a string)", line: 0, column: 0)', src/vm_controller.rs:712:45
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test vm_controller::tests::check_flatten ... FAILED

After removing serde(flatter) for String data, it does not generate any error.

I am wondering if there is any bug at generating enum ObjectOptions, Or do I misunderstand how to use it?

arcnmx commented 2 years ago

Yeah this was fallout from struct reorganization, try out e00c3e85de980075a43bf82e8478224a266200b5 which should fix this.

gurugio commented 2 years ago

Great. I will try and ping you if it doesn't work. Thank you very much.

gurugio commented 2 years ago

Hi,

I have a problem.

We are using v0.8.0 because we use Qemu 6.x version. Qapi-rs v0.10.0 does not have a backward compatibility with Qemu 6.x version. (Some memory device types and etc were changed.)

Could you please do back-porting the patch into 0.8.0?

I am able to do back-porting with only cherry-picking. There is no conflicts at all. So I guess it doesn't take long time.

arcnmx commented 2 years ago

Yeah, I was planning on backporting it once it was tested/released anyway, try this: 9eb901b2b22460f775a95b41d29b1c8cf86a5295

gurugio commented 2 years ago

I tested the branch v0.8.x and got no error. Thank you very much.

arcnmx commented 2 years ago

Thanks for the confirmation, updates for all branches should be on crates.io now

gurugio commented 2 years ago

Hi, I saw 0.9.1 but there is no other version upgrade. Could you please push 0.8.1?

arcnmx commented 2 years ago

qapi-qmp 0.8 depends on qapi-codegen 0.6, which contains the update. A cargo update should pull it in automatically.

gurugio commented 2 years ago

Hi,

Yes, all is well with qapi 0.8.0. Thank you very much.