arcnmx / qapi-rs

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

why struct BlockdevOptions generated by build.rs in the qmp.rs file has #[serde(flatten)] attribute? #16

Closed flyflypeng closed 1 year ago

flyflypeng commented 1 year ago

I am confused why struct BlockdevOptions generated by build.rs in the qmp.rs file has #[serde(flatten)] attribute, which generated json string like this:

{"driver":"file","cache":{"direct":true},"read-only":false,"node-name":"drive-0","filename":"/dev/dm-8"}

instead of the following structured format describled in the qmp manual:

##
# @blockdev-insert-medium:
#
# Inserts a medium (a block driver state tree) into a block device. That block
# device's tray must currently be open (unless there is no attached guest
# device) and there must be no medium inserted already.
#
# @id: The name or QOM path of the guest device
#
# @node-name: name of a node in the block driver state graph
#
# Since: 2.12
#
# Example:
#
# -> { "execute": "blockdev-add",
#      "arguments": {
#          "node-name": "node0",
#          "driver": "raw",
#          "file": { "driver": "file",
#                    "filename": "fedora.iso" } } }
# <- { "return": {} }
#
# -> { "execute": "blockdev-insert-medium",
#      "arguments": { "id": "ide0-1-0",
#                     "node-name": "node0" } }
#
# <- { "return": {} }
#
##
arcnmx commented 1 year ago

You may need to share your code, I think you're running into the same mistake as described in https://github.com/arcnmx/qapi-rs/issues/14#issuecomment-1201346637 That example in the documentation seems to be using two blockdevs: a BlockdevOptions::File inside a BlockdevOptions::Raw

flyflypeng commented 1 year ago

I create blockdev_add type instance by the following function:

fn to_blockdev_add(&self) -> blockdev_add {
        blockdev_add(BlockdevOptions::file {
            base: BlockdevOptionsBase {
                node_name: Some(self.id.to_string()),
                read_only: self.readonly,
                auto_read_only: None,
                cache: Some(BlockdevCacheOptions {
                    no_flush: None,
                    direct: Some(true),
                }),
                force_share: None,
                discard: None,
                detect_zeroes: None,
            },
            file: BlockdevOptionsFile {
                drop_cache: None,
                locking: None,
                x_check_cache_dropped: None,
                filename: self.file.as_ref().unwrap().to_string(),
                aio: None,
                pr_manager: None,
            },
        })

and I write a simple test function to print the blockdev_add qmp arguments in json style:

 #[test]
    fn test_generate_block_device_qmp_commands() {
        let virtio_blk_device = VirtioBlockDevice::new(
            VIRTIO_BLK_DRIVER,
            "drive-0",
            Some("/dev/dm-8".to_string()),
            Some(false),
        );

        let blockdev_add_qmp_cmd = virtio_blk_device.to_blockdev_add().0;
        let blockdev_add_qmp_json_str = serde_json::to_string(&blockdev_add_qmp_cmd).unwrap();
        println!(
            "blockdev_add qmp cmd json string: {}",
            blockdev_add_qmp_json_str
        );
    }
flyflypeng commented 1 year ago

You may need to share your code, I think you're running into the same mistake as described in #14 (comment) That example in the documentation seems to be using two blockdevs: a BlockdevOptions::File inside a BlockdevOptions::Raw

I am just using BlockdevOptions::file type

arcnmx commented 1 year ago

I am just using BlockdevOptions::file type

The documentation example JSON you quoted looks like it's using both types. What JSON output do you expect instead of what you're getting? If you want output like what's in the example, you'll want something like...

blockdev_add(BlockdevOptions::raw {
  raw: BlockdevOptionsGenericFormat {
    file: BlockdevRef::definition(Box::new(BlockdevOptions::file { etc })),
  }.into(),
})
flyflypeng commented 1 year ago

I get it! Thanks a lot ~